8154956: Module system implementation refresh (4/2016)

[Peter Levart]

Hi,

Last minute findings...

In ModulePath, jarPackages() and jmodPackages() helper method both use 
toPackageName(ZipEntry entry) method which checks for "classes/" prefix 
and strips it away. But "classes" is a valid package name or prefix (in 
case it is found in a .jar file). So it's better to only strip it off 
for jmodPackages()...

http://cr.openjdk.java.net/~plevart/jdk9-jake/ModuleSystem.referesh.201604/webrev.02/

Regards, Peter

On 05/02/2016 12:27 AM, Peter Levart wrote:
> Hi,
>
> I have some mostly stylish changes (which you can use or not) 
> regarding usage of Optional and Stream(s) in package java.lang.module:
>
> http://cr.openjdk.java.net/~plevart/jdk9-jake/ModuleSystem.referesh.201604/webrev.01/
>
> ...with some optimizations/cleanups/fixes:
>
> ModuleFinder.compose:
> - the composed ModuleFinder.findAll() is more efficient this way as it 
> doesn't need to call back to find().
>
> ModulePath:
> - there's unneeded overhead to call distinct() for a Stream that is 
> later collected into Collectors.toSet() as the collector already 
> uniquifies the elements.
> - lines 432...437 (old) - you don't want to treat entries in 
> SERVICES_PREFIX directory that end with ".class" as classFiles - so I 
> changes the:
>         .filter(e -> (e.endsWith(".class") || 
> e.startsWith(SERVICES_PREFIX)))
>   to:
>         .filter(s -> (s.endsWith(".class") ^ 
> s.startsWith(SERVICES_PREFIX)))
>
> ModuleReader:
> - the default method read(String name) should close the InputStream 
> after reading from it, shouldn't it?
>
> ModuleReferences:
> - no need for 'closed' flag in SafeCloseModuleReader to be volatile as 
> it is always accessed under lock (either read lock when reading or 
> write lock when writing).
>
> Resolver:
> - Implemented caching of ResolvedModule(s) as they are entered into 
> resolved graph so that there are no duplicates.
> - Simplified last stage of iterative algorithm to build resolved 
> graph. There's no need to construct a map of changes and apply it 
> afterwards to 'g1' as changes can be applied individually for each 
> module 'm1' in the graph 'g1' as they are collected.
>
>
> Regards, Peter
>
> On 04/29/2016 02:38 PM, Alan Bateman wrote:
>>
>> There have several changes in jake that we need to bring into 
>> jdk9/dev. The main changes are:
>>
>> 1. The policy described in JEP 261 for the root modules to resolve 
>> when compiling code in the unnamed module or where the main class is 
>> loaded from the class file. This is a disruptive change and we need 
>> to get through the transition.
>>
>> Related is a new token `ALL-DEFAULT` that can be specified to 
>> `-addmods` to resolve the same roots when the initial module is a 
>> named module. This will eventually replace `ALL-SYSTEM` but we can't 
>> remove that just yet. The launchers for javac, javadoc, jlink and a 
>> few other tools that load arbitrary code in unnamed modules are now 
>> compiled with this option.
>>
>> 2. The transition to the new form of -Xpatch. This is mostly changes 
>> in hotspot but there are changes in other repos too to drop or 
>> replace the old form of -Xpatch (boot cycle builds for example).
>>
>> 3. Removal of the old form of -XaddExports and -XaddReads. This has 
>> build + test changes.
>>
>> 4. The second phase of integrity hashing. With this phase then the 
>> build records in java.base the hashes of the non-upgradeable modules. 
>> This prevents accidental linking of standard/JDK modules from 
>> different JDK builds. The jar and jmod tools have updated options to 
>> support this.
>>
>> 5. Peter Levart's patch to replace the internal WeakSet in jlr.Module 
>> with a WeakKeyPair.
>>
>> 6. Updates to jlink option handling. The reason this went into 
>> jdk9/dev is that it is disruptive to FX packager and it's hard to 
>> coordinate with FX when it's a separate forest. The packager class 
>> that Chris Bensen has added to jlink will allows us to iterate on 
>> jlink with reduced risk of breaking FX.
>>
>> There are several small bug fixes and clean-ups in several areas, 
>> we'll have a lot more of these for the next update.
>>
>> One other thing to mention is that we've bumped the required version 
>> of jtreg as jtreg relies on-Xpatch to add test cases into modules and 
>> so it needed to be updated too.
>>
>> The webrevs, all repos are here:
>> http://cr.openjdk.java.net/~alanb/8154956/1/
>>
>> There are a couple of files in the webrevs that we probably won't 
>> bring to JDK 9 in this update:
>>
>> i. We have a patch to IDL compiler in the corba repo that needs a 
>> more extensive fix.
>> ii. The javadoc change in ModuleFinder as there are still details to 
>> decide on how modular JARs work as multi-release JARs.
>>
>> One other point is that the webrevs are against jdk-9+116 for now. 
>> I've done a test merge + build with the current jdk9/dev forest and 
>> there are only a few conflicts to fix. I will re-merge + test with 
>> jdk9/dev once we have agreed the changes for this update.
>>
>> Finally, just to say that we'll probably continue in jake for a while 
>> after we get through this update. There are several design issues on 
>> the JSR issues list that will likely require a few iterations and a 
>> bit of bake time before we bring them to JDK 9.
>>
>> -Alan
>