RFR: 8316969: Improve CDS module graph support for --module option [v3]

[Calvin Cheung]

On Fri, 20 Oct 2023 14:34:52 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   simplify some code in modules.cpp
>
> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 472:
> 
>> 470: 
>> 471:         // If -Xshare:dump and mainModule are specified, check if the mainModule
>> 472:         // is in the runtime image and not in the upgrade module path. If so,
> 
> Small typo here, should be "on the upgrade module path" rather than "in".

Fixed

> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 481:
> 
>> 479:                     canArchive = true;
>> 480:                 }
>> 481:             }
> 
> An alternative to avoid the 3 levels of if expressions is to use:
> 
>         String scheme = systemModuleFinder.find(mainModule)
>                 .stream()
>                 .map(ModuleReference::location)
>                 .flatMap(Optional::stream)
>                 .findAny()
>                 .map(URI::getScheme)
>                 .orElse(null);
>         if ("jrt".equalsIgnoreCase(scheme)) {
>             canArchive = true;
>         }

I tried the above but got the following build error:

Optimizing the exploded image
Error occurred during initialization of boot layer
java.lang.NullPointerException
ExplodedImageOptimize.gmk:39: recipe for target '/scratch/cccheung/ws/git/curr_jdk/build/mywork-debug/jdk/_optimize_image_exec.marker' failed

Let's keep the existing code for now because I may need to refactor the code into a utility method for a upcoming RFE to support the --add-modules option.

> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 491:
> 
>> 489:                                         cf,
>> 490:                                         clf);
>> 491:             if (!hasSplitPackages) {
> 
> Did you mean to drop the hasIncubatorModules check? Incubator modules are not resolved by default.

Yes, because of the following code further up in the same method:

            if (!haveModulePath && addModules.isEmpty() && limitModules.isEmpty()) {
                systemModules = SystemModuleFinders.systemModules(mainModule);
                if (systemModules != null && !isPatched) {
                    needResolution = (traceOutput != null);
                    canArchive = true;
                }
            }
            if (systemModules == null) {
                // all system modules are observable
                systemModules = SystemModuleFinders.allSystemModules();
            }

`SystemModuleFinders.systemModules(mainModule)` returns null if `mainModule` is not null (I'm not sure why).
Then, the `SystemModuleFinders.allSystemModules()` returns the generated class `SystemModules$all` with the following method:

    public boolean hasIncubatorModules() {
        return true;
    }


Also, there's the following change:
`needResolution = (traceOutput != null) || CDS.isDumpingStaticArchive();`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1367457252
PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1367457043
PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1367455686