RFR: 8220633: Optimize CacheFSInfo

Jonathan Gibbons jonathan.gibbons at oracle.com
Tue Mar 26 18:21:52 UTC 2019


Liam,

I looked again at both webrev.00 and webrev.01.  I think there is a 
better solution than both.

The original code has the explanatory comment; for webrev.01, "form 
follows function" comes to mind: while being more idiomatic, using 
computeIfAbsent, it is also more verbose and thus less clear.

How about a variant of webrev.00 in which you do lock the newly-added 
specific cache, and thus do away with the comment.

   91     @Override
   92     public List<Path> getJarClassPath(Path file) throws IOException {
              synchronized (jarClassPathCache) {
96 List<Path> jarClassPath = jarClassPathCache.get(file);
97 if (jarClassPath == null) {
98 jarClassPath = super.getJarClassPath(file);
99 jarClassPathCache.put(file, jarClassPath);
100 }
101 return jarClassPath; }
  102     }

The reformatting on lines 55-57 seems a bit gratuitous, especially 
putting the cast on a line by itself.

The rest seems OK.

- Jon


On 03/26/2019 08:50 AM, Liam Miller-Cushon wrote:
> Is there any other feedback on the change?
>
> On Wed, Mar 13, 2019 at 9:29 PM Liam Miller-Cushon <cushon at google.com 
> <mailto:cushon at google.com>> wrote:
>
>     On Wed, Mar 13, 2019 at 5:59 PM Jonathan Gibbons
>     <jonathan.gibbons at oracle.com <mailto:jonathan.gibbons at oracle.com>>
>     wrote:
>
>         I could go either way on getJarClassPath.  The initial version
>         in webrev.00 was definitely shorter.
>
>     computeIfAbsent feels a little more idiomatic to me than the
>     intentional race (and the comment to explain why it's safe), but I
>     don't really have a preference.
>
>     I'm happy to revert to webrev.00 if you prefer.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190326/6a478b45/attachment-0001.html>


More information about the compiler-dev mailing list