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

[Jan Lahoda]

On 25.1.2017 13:59, Claes Redestad wrote:
>
>
> 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?

If the jar contains a new package specific for e.g. version 9, listing 
of that package won't work (both before or after this patch). Not 
completely sure if that's intended, but I think that's something that 
should be solved in the JarFS. (Adding Steve on CC.)

Jan

>
> 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
>>>
>