[lworld] RFR: 8366200: Modify jimage package flags to support preview mode [v2]

[David Beaumont]

On Fri, 29 Aug 2025 13:33:11 GMT, David Beaumont <duke at openjdk.org> wrote:

>> Reuses the previously unused package flags in the /packages/xxx entries of jimage files to support preview mode.
>> 
>> With these flags it becomes simple to determine things like:
>> 1. Which modules are children of /packages/xxx
>>   - This can differ between preview and non-preview mode.
>> 
>> 2. Which modules/packages have any preview content
>>   - Useful in preview mode for faster rejection testing to avoid double-lookup on all resources.
>> 
>> If there are no preview resources built into the jimage file, then the difference between output is that the "isEmpty" flag (old version) has become a "hasContent" flag (but since these flags are not read by anyone right now, this should not be an issue).
>> 
>> Note that bumping the minor version number was done to ensure that, during testing, no undiscovered code is reading the new file with old logic (this would work since there's no structural change, but the semantics are different).
>> 
>> This does mean that on systems where a 'jimage' binary is installed for the JDK (e.g. /usr/bin/jimage), that tool will stop working on files generate by this code, but the version of that binary in the built JDK will work. The alternative is to just not bump the version number (if nobody is reading the flags now, it shouldn't matter what's in them).
>
> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Minor tidy

Some pre-loaded comments for reviewers.

src/java.base/share/classes/jdk/internal/jimage/ImageHeader.java line 33:

> 31: 
> 32: /**
> 33:  * Defines the header and version information for jimage files.

Technically optional even though this is a semantic change (it's a change in data nobody should be reading at the moment).

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 48:

> 46: public final class ImageResourcesTree {
> 47:     public static boolean isTreeInfoResource(String path) {
> 48:         return path.startsWith("/packages/") || path.startsWith("/modules/");

Fixed in passing. Previously this would silently hide/ignore packages with names starting "packages..." or "modules..." in the jimage tool.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 178:

> 176:             private final int flags;
> 177: 
> 178:             PackageReference(String name, int flags) {

I really don't like the name "PackageReference", since this does not reference a package, it references a module in which the package exists. I'd be happy to just refactor the name to something else but didn't want to do too many "preferential" changes.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 183:

> 181:             }
> 182: 
> 183:             String name() {return name;}

Methods here make the comparator easier to construct.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 197:

> 195:             @Override
> 196:             public String toString() {
> 197:                 return String.format("%s [has_normal_content=%s, has_preview_content=%s, is_preview_only=%s]",

I assume nobody was expected to be relying on the `toString()` output.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 206:

> 204: 
> 205:         // Outside this class, callers should access via modules() / moduleCount().
> 206:         private final Map<String, PackageReference> unsortedReferences = new HashMap<>();

Changing the name to make it clear this is not the preferred way to read references now. If these classes were in separate sources it would be easier to encapsulate.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 243:

> 241:         private void validate() {
> 242:             // If there's a module for which this package has content, it should be first and unique.
> 243:             if (modules().skip(1).anyMatch(ref -> !ref.isEmpty())) {

With module references sorted, the validation becomes easier.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 498:

> 496:                 buff.order(writer.getByteOrder());
> 497:                 pkgNode.modules().forEach(mod -> {
> 498:                     buff.putInt(mod.flags);

This is the whole point of this change, to write the new flags into the package entry.

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

PR Review: https://git.openjdk.org/valhalla/pull/1537#pullrequestreview-3168722127
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310152805
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310168105
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310172648
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310177352
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310178660
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310181114
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310183290
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2310189310