RFR: JDK-8171294: Slow compilation with long classpaths under JDK 9

[Claes Redestad]

On 01/25/2017 01:27 PM, Jan Lahoda wrote:
> On 25.1.2017 12:29, Maurizio Cimadamore wrote:
>>
>>
>> On 25/01/17 10:27, Claes Redestad wrote:
>>> Hi,
>>>
>>> this might be a pre-existing issue, but doesn't this need to take 
>>> Multi-
>>> Release JARs into account and walk the (most) appropriate subtree?
>> My understanding (Jan corrects me if I'm wrong) is that javac delegates
>> MR-jar questions to the underlying nio file system. So, given the cache
>> is implemented using a nio file walker, the code should be already doing
>> the appropriate thing?
>
> Yes, we defer the MR-jar handling to the JarFileSystem. I am not sure 
> if the handling there should be improved, but I don't think this patch 
> changes anything. We only keep (valid) package names, and while we 
> will currently only get packages in the "default" version, I believe 
> listing also works only for such packages currently.

Right, it should work for the default case of compiling at the current JDKs
runtime level, and there's code in BaseFileManager that allows setting 
of the
"multi-release" attribute that'd instruct the JarFileSystem to do the right
thing when compiling for another release...

What I was wondering about is whether the env per jar file is set 
appropriately,
i.e., does --source/--target mean we're compiling against the 
appropriate code,
and - with this patch - do we see the appropriate set of packages in 
each jar?

Sorry for going out on a tangent here, the patch itself looks really 
good to me. :-)

Thanks!

/Claes

>
> Thanks,
>     Jan
>
>>
>> Maurizio
>>>
>>> Thanks!
>>>
>>> /Claes
>>>
>>> On 2017-01-24 15:35, Jan Lahoda wrote:
>>>> Hi,
>>>>
>>>> As reported:
>>>> https://bugs.openjdk.java.net/browse/JDK-8171294
>>>> http://mail.openjdk.java.net/pipermail/compiler-dev/2016-December/010596.html 
>>>>
>>>>
>>>>
>>>>
>>>> javac has a problem with compiling with many jars on classpath. The
>>>> problem is twofold:
>>>> a) there is JavacFileManager.ArchiveContainer.pathCache, and this 
>>>> cache
>>>> keeps entries not only for path that are in the given archive/jar, but
>>>> also for those that are not in it, which may consume too much memory
>>>>
>>>> b) when listing a package, for each archive/jar, an equivalent of
>>>> Files.exists(Path.resolve(String)) is done, which takes some time, and
>>>> this is repeated a lot of times (and most of the time, the package 
>>>> does
>>>> not exist in the given jar).
>>>>
>>>> The proposed patch is basically Maurizio's patch that lists 
>>>> packages on
>>>> opening and then can quickly decide if a given archive/jar contains 
>>>> the
>>>> given package or not. The biggest change from the original patch is 
>>>> that
>>>> the packages are computed immediately when the ArchiveContainer is
>>>> created, as opposed to compute them lazily. The reason is that the
>>>> Containers are created lazily, and are used immediately after being
>>>> created.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~jlahoda/8171294/webrev.00/
>>>>
>>>> How does this look?
>>>>
>>>> Thanks,
>>>>     Jan
>>