RFR 8220792: JavacFileManager.list() performance

[Ron Shapiro]

- Added a comment explaining the ordering. LMK if you have a better way of
explaining it, as I think you grok it better than I do
- Thanks for the pointer to javacFileManagerOptions. I updated to use that.
Both are in this updated webrev:
http://cr.openjdk.java.net/~ronsh/8220792/webrev.02/

- Regarding indexing: I don't think today anything would detect updates to
a jar file if a new RelativeDirectory was added. Perhaps that's
intentional, because we don't care/don't even want to try to support
someone updating their jarfile in the middle of a compilation. I defer to
others about indexing other containers - it looks like the
JRTImageContainer depends on the JRTIndex class, which does a lazy
computation of the index, so it looks like it would be some additional work
to add that to the index. But perhaps it's not even worth it - IIUC, there
should only be one JRTImageContainer, and probably a limited amount of
DirectoryContainers - the performance issue that I was observing was that
with many jars, each needed to do some work for every call to list. We've
now reduced O(O(JRTImageContainer = 1) + O(DirectoryContainer = likely to
be small) + O(ArchiveContainer = possibly large)) per list() call
to O(O(JRTImageContainer = 1) + O(DirectoryContainer = likely to be
small) + O(overlapping ArchiveContainers, which is hopefully small))



On Wed, Mar 27, 2019 at 1:47 PM Jonathan Gibbons <
jonathan.gibbons at oracle.com> wrote:

>
> On 3/27/19 9:06 AM, Ron Shapiro wrote:
> >
> > > It's not clear to me why you maintain the list of PathAndContainer
> > as a sorted list, and hence why you need to use java.util.List instead
> > of javac's List.
> > There are some tests that ensure that the classpath that javac sees is
> > equivalent to the classpath the JVM sees vis-a-vis ordering. Without
> > sorting, javac was seeing a different order, and I presume that is
> > because the JRTImageContainer and DirectoryContainers could appear
> > anywhere. Sorting solved that issue.
>
> OK, I think I understand the issue and why sorting fixes it.
>
> While some tests may check that "the classpath that javac sees is
> equivalent to the classpath the JVM sees vis-a-vis ordering", that's not
> the primary issue. The primary issue is that the new cache must preserve
> the order in which containers are searched on the classpath (i.e just
> within javac, separate from any part set by the JVM). While this is not
> so important when looking up classes in "well-behaved" packages, it is
> important when handling split packages and resources, where the contents
> in one container may hide the contents in another container.
>
> I think this is subtle enough that it is worth a comment in the code,
> explaining that it is important to preserve the search order used in the
> uncached Location path.
>
> -- Jon
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190329/a1b14860/attachment-0001.html>