On Wed, 5 Nov 2025 18:35:49 GMT, David Beaumont <[email protected]> wrote:
>> Adds support for writing preview related flags into jimage files.
>>
>> Preview mode is complex. It's not nearly as simple as "does something in
>> /modules/xxx/... have an entry in /modules/xxx/META-INF/preview/...".
>>
>> Specific issues include:
>>
>> Supporting preview-only resources without forcing a double lookup on
>> everything.
>> Changing the set of entries in /packages/xxx directories to account for
>> preview only packages in some modules.
>> Minimising the work done during image reader initialization to only need to
>> process the small number of preview resources (rather than scanning the
>> whole file to look for them).
>> The new flags added by this code address these issues, but calculating them
>> correctly with only minor adjustments to the existing code was not feasible,
>> it just became a lot larger and very complex.
>>
>> To address this, a new type (ModuleReference) is introduced to track and
>> then merge information about packages seen in each module. This allows a
>> much simpler inner loop for processing resource paths when building the node
>> tree, combined with a subsequent merging stage to produce the final package
>> information for each module.
>>
>> Not that since ModuleReference is needed during jimage reading, that class
>> is already present in the previous PR on which this is based, but it starts
>> to be used to calculate the module flags in this PR.
>>
>> This PR can also adds the ImageReader unit tests for preview mode, which
>> rely on being able to generate jimage files with preview mode flags in.
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Feedback and test fix
Warts included, should work as intended.
src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java line 177:
> 175: * @return package flags for {@code "/packages/xxx"} directory
> entries.
> 176: */
> 177: public static int getPackageFlags(List<ModuleReference>
> moduleReferences) {
This exclusively works on ModuleReferences, it seems that it should be in
ModuleReference, not ImageLocation. It is only in ImageLocation because the
flags are private.
And it is only used by ImageResourceTree, so should be there.
There has to be a better structure that doesn't cause so much confusion and
crossing boundaries.
And yes, I think it is another example where having similar but different flags
is harmful to the structure and meaning of the code.
-------------
Marked as reviewed by rriggs (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/1718#pullrequestreview-3424569473
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1718#discussion_r2496272604
