RFR: JDK-8193717: Import resolution performance regression in JDK 9 (original) (raw)

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Mon May 21 11:44:34 UTC 2018

Few comments:

the use of an array as a value in the map seems to cause too much GC
overhead - e.g. the array has to be re-allocated from scratch each
time a new subscope is added for a given name; wouldn’t a List be
better? I assume that element count will be low, so the linked list
overhead might be negligible (and better than allocating a
bigger-than-needed array, as for an ArrayList).
this code looks dubious:

|Scope[] scopes = name2Scopes.getOrDefault(name, new Scope[0]); + if (scopes == null) + return Collections.emptyList(); |

When can ‘scopes’ ever be null? If there’s no entry for given name, the use of getOrDefault guarantees that a zero-element array is returned; so the only option would be if ‘null’ is explicitly stored as a map value; but looking at the code, the stores are done here:

|Scope[] existing = name2Scopes.get(name); + if (existing != null) + existing = Arrays.copyOf(existing, existing.length + 1); + else + existing = new Scope[1]; + existing[existing.length - 1] = newScope; + name2Scopes.put(name, existing); |

So, again, this should guarantee that the value associated with a ‘put’ is always non-null?

Maurizio

On 21/05/18 12:24, Jan Lahoda wrote:

Hi,

A webrev updated to the current situation, and with a comment explaining the need for the HashMap is here (as suggested offline): http://cr.openjdk.java.net/~jlahoda/8193717/webrev.01/ Any feedback is welcome. Thanks, Jan On 2.3.2018 17:52, Jan Lahoda wrote: Hi,

Having a source with a lot of imports can lead to a long compilation time, as the imports are lazily searched on each query to the Scope. But at least for named imports, the name is known, and can be used for quick filtering. (Sadly, for on-demand imports/package imports, this is much more difficult, and this patch does not improve them.) The principle of the patch is to have a map from name to scopes that may contain the given name in Scope.NamedImportScope. Also, the "current-file top-level scope" (JCCompilationUnit.toplevelScope) is no longer appended to the named import scope, but rather the relevant lookup is enhanced to query it as well if needed. This seems cleaner that trying to append the scope to the fast name lookup. Bug: https://bugs.openjdk.java.net/browse/JDK-8193717 Webrev: http://cr.openjdk.java.net/~jlahoda/8193717/webrev.00/ Liam, could you please check if this helps with your real-world case? How does this look? Thanks, Jan

​ -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180521/32417a54/attachment.html>

More information about the compiler-dev mailing list