> 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.

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.


- Madhan


-----------------------------------------------------------
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
> 
>

Reply via email to