On Fri, 13 Dec 2024 22:42:47 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Henry Jen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move up Snippet setup as a builder
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
> line 584:
>
>> 582: private void dedups(ModuleDescriptor md) {
>> 583: // exports
>> 584: for (Exports e : sorted(md.exports())) {
>
> Is sorting needed? de-duplicating shouldn't depend on the order of the input,
> should it?
I will need to check again, this was added when we need to ensure the sequence
of cache from the dedupSet to reproduce same image.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
> line 619:
>
>> 617: // generate dedup set fields and provider
>> methods
>> 618: var dedupSets = genConstants(clb);
>> 619:
>
> These constants are all for building the module descriptors and `dedupSets`
> is needed by `genModuleDescriptorsMethod`. This can be moved and be
> called in `genModuleDescriptorsMethod`.
The static initializer currently only contains the cache from DedupSet, but it
feels wrong to generate the static initializer in genModuleDescriptorsMethod.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1884647324
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1884648803
