> On Jan. 26, 2017, 9:53 p.m., Madhan Neethiraj wrote: > > common/src/main/java/org/apache/atlas/utils/LruCache.java, line 33 > > <https://reviews.apache.org/r/55443/diff/7/?file=1616739#file1616739line33> > > > > Consider using LinkedHashMap<K, V> to implement LruCache: > > > > public class LruCache<K, V> extends LinkedHashMap<K, V> { > > static final float LRU_LOAD_FACTOR = 0.75f; > > > > private final int maxCapacity; > > > > public LruCache(int maxCapacity) { > > super(maxCapacity, LRU_LOAD_FACTOR, true); > > > > this.maxCapacity = maxCapacity; > > } > > > > @Override > > protected boolean removeEldestEntry(Map.Entry eldest) { > > return size() > maxCapacity; > > } > > } > > > > More details at: > > http://docs.oracle.com/javase/7/docs/api/java/util/LinkedHashMap.html#removeEldestEntry(java.util.Map.Entry) > > Jeff Hagelberg wrote: > I've changed the implementation of LruMap to use LinkedHashMap as the > backing store. It would take a lot of work to make LruMap extend > LinkedHashMap directly. The main obstacle is that some of the the methods > require the last accesss time, and LinkedHashMap does not provide that. This > at least avoids having to maintain our own linked list implementation. > > Madhan Neethiraj wrote: > Jeff - I suggested using LinkedHashMap to avoid having to deal with > complicated/error-prone lock/unlock in our code. Is access to lastAccessTime > important here, given that LinkedHashMap automatically purges least recently > accessed entries? > > The implemtation in LruCache and LruMap are not needed if LruCache > extends from LinkedHashMap as suggested in my earlier comment.
Well, the access times are only needed for the reaping functionality. If that is not needed, than we could probably replace LruMap with a LinkedHashMap. LinkedHashMap does not look like it is thread safe. We could simplify the logging by wrapping it with a SynchronizedMap though. Yeah, probably the reaping is not needed, that is disabled by default anyways and really does not add much. - Jeff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55443/#review163176 ----------------------------------------------------------- On Jan. 27, 2017, 5:44 p.m., Jeff Hagelberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55443/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2017, 5:44 p.m.) > > > Review request for atlas, David Kantor and Neeru Gupta. > > > Bugs: ATLAS-1387 > https://issues.apache.org/jira/browse/ATLAS-1387 > > > Repository: atlas > > > Description > ------- > > Adds a compiled query cache to Atlas. This avoids the overhead of parsing > and translating DSL queries that have been previously executed. > > > Diffs > ----- > > common/pom.xml 0226541b2dd73e8a01f050982cb8b37f5fed9cab > common/src/main/java/org/apache/atlas/utils/LruCache.java PRE-CREATION > common/src/main/java/org/apache/atlas/utils/LruMap.java PRE-CREATION > common/src/test/java/org/apache/atlas/utils/LruCacheTest.java PRE-CREATION > distro/src/conf/atlas-application.properties > 303ce7b00c93f2499e7b18c5f3a131b50c150b69 > > repository/src/main/java/org/apache/atlas/discovery/graph/GraphBackedDiscoveryService.java > fb488cdfc7cef195ffc5221afb9a9109a6e92fc9 > > repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java > 6655085ee11c94addca1564cf77bbdb001c4586f > repository/src/main/java/org/apache/atlas/util/CompiledQueryCacheKey.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/util/NoopGremlinQuery.java > PRE-CREATION > repository/src/main/scala/org/apache/atlas/query/QueryProcessor.scala > 5693c9ecc1a96154a8b0ac48256e5074dc041c5f > > repository/src/test/java/org/apache/atlas/util/CompiledQueryCacheKeyTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/55443/diff/ > > > Testing > ------- > > Ran all atlas tests, no regressions found. There were some tests that failed > both with and without these changes. > > > Thanks, > > Jeff Hagelberg > >
