[lworld] RFR: 8366200: Modify jimage package flags to support preview mode [v4]
Roger Riggs
rriggs at openjdk.org
Thu Sep 4 20:42:56 UTC 2025
On Fri, 29 Aug 2025 15:27:21 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:
>
> More tests and test tweaking for clarity
Looks pretty thorough.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 154:
> 152: * entry to determine if that module contains resources for the package.
> 153: *
> 154: * <p>If all entries are marked with {@code IS_PREVIEW_ONLY}
Is there more to this comment?
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]",
A bit verbose for output, very few will see or use.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 230:
> 228: // It is possible (but not worth checking for) that these flags are the same
> 229: // as the existing reference (e.g. updating with an empty preview package).
> 230: unsortedReferences.put(moduleName, new PackageReference(moduleName, flags));
is the any cleaner using `compute` or `computeIfPresent`, etc.
test/jdk/tools/jlink/whitebox/jdk.jlink/jdk/tools/jlink/internal/ImageResourcesTreeTest.java line 61:
> 59: "/java.base/java/util/resource.txt",
> 60: "/java.logging/java/util/logging/LoggingClass.class",
> 61: "/java.logging/java/util/logging/OtherLoggingClass.class");
Would these work well using @ParameterizedTest? And other tests below that use the same lists.
Or should these lists be shared.
-------------
PR Review: https://git.openjdk.org/valhalla/pull/1537#pullrequestreview-3186990961
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2323366727
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2323376999
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2323389797
PR Review Comment: https://git.openjdk.org/valhalla/pull/1537#discussion_r2323483229
More information about the valhalla-dev
mailing list