RFR: 8319343: Improve CDS module graph support for --add-modules option [v2]

Calvin Cheung ccheung at openjdk.org
Mon Oct 21 16:32:44 UTC 2024


On Mon, 21 Oct 2024 13:17:10 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Hi Alan,
>> I tried your suggestion but it can't handle the `ALL-SYSTEM` case.
>> I made some slight adjustments to your patch as follows:
>> 
>> 
>>     /**
>>      * Returns true if all modules named in the given set are in the Configuration and
>>      * the run-time image.
>>      */
>>     private static boolean allJrt(Configuration cf, Set<String> moduleNames) {
>>         if (moduleNames.size() == 1 && moduleNames.contains(ALL_SYSTEM)) {
>>             return true;
>>         }
>>         return !moduleNames.stream()
>>                 .filter(mn -> !mn.equals(ALL_SYSTEM))
>>                 .map(mn -> cf.findModule(mn).orElseThrow())
>>                 .map(m -> m.reference().location().orElseThrow())
>>                 .anyMatch(uri -> !uri.getScheme().equalsIgnoreCase("jrt"));
>>     }
>> 
>> What do you think?
>
> Good point as ALL-SYSTEM and ALL-MODULE-PATH are allowed in the value. I assume you don't need to special case the single value case as this is dump time so not performance critical.

I've removed the `if` check at the beginning of the method.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21553#discussion_r1809136528


More information about the core-libs-dev mailing list