[GitHub] [maven-resolver] rjatkins commented on issue #29: [MRESOLVER-68] Add cache around TrackingFileManager.getLock(File)

[GitBox <gi...@apache.org>]

rjatkins commented on issue #29: [MRESOLVER-68] Add cache around TrackingFileManager.getLock(File)
URL: https://github.com/apache/maven-resolver/pull/29#issuecomment-478428109
 
 
   #22 adds full caching around all uses of the properties files tracked by TrackingFileManager, while this PR was focused only on the calculations of canonical paths, where the bulk of the time is spent in my local testing. Caching all properties reads will give further performance improvements - my testing shows another 130s saved on my worst case maven project.
   #22 uses an inline implementation of caching, while this PR uses Guava's CacheBuilder to maintain the cache for us, which includes thread-safety guarantees, and bulk expired entry cleanup after reads and writes. The canonical path cache in #22 uses an unbounded, never invalidated cache, while this PR ensures we only cache at most 1024 entries (and that's configurable). With the update I just pushed, it also caches at most 1024 properties objects too.
   I have no strong opinion on either implementation - if you're willing to gain a code level dependency on Guava here, then I'm hoping it's easier to understand the code in this PR. Otherwise, the cache in #22 looks good enough. I'm just eager to see either of them merged, so we can all gain from the performance improvements.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services