RFR: 8368467: [lworld] Add new flag generation for jimage to support preview mode [v3]
Roger Riggs
rriggs at openjdk.org
Mon Sep 29 01:53:57 UTC 2025
On Tue, 23 Sep 2025 23:07:47 GMT, David Beaumont <duke at openjdk.org> 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.
>>
>> Compare and review this against https://github.com/openjdk/valhalla/pull/1619.
>
> David Beaumont has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains four new commits since the last revision:
>
> - Restoring lost changes and updating some comments.
> - add system property guard to preview mode
> - jimage writer changes to support preview mode.
>
> * Remove TODOs now jimage version is bumped
> * jimage writer changes to support preview mode.
> - Manually deleting ImageReaderFactory (it returned somehow)
src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java line 445:
> 443: // Only 2 choices, either the "/modules" or "/packages" root.
> 444: assert isRootDir() : "Invalid root directory: " + getFullName();
> 445: return (getFlags() & FLAGS_IS_PACKAGE_ROOT) != 0
Introducing a flag bit that is carried around in lots of entries but applies to only one is overkill.
The simple test above for the first letter is cleaner and very localized.
src/java.base/share/classes/jdk/internal/jimage/PreviewMode.java line 68:
> 66: if (!ENABLE_PREVIEW_MODE) {
> 67: return false;
> 68: }
As commented in the 1618 PR, the extra subclasses caused by the overrides can be replaced by the resolve method switching on `ordinal()`.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageLocationWriter.java line 58:
> 56: static ImageLocationWriter newLocation(String fullName,
> 57: ImageStringsWriter strings,
> 58: long contentOffset, long compressedSize, long uncompressedSize, int previewFlags) {
Wrap or untab to avoid long lines.
test/jdk/jdk/internal/jimage/ImageReaderTest.java line 107:
> 105: "/modules/modfoo/com/foo/bar"})
> 106: public void testModuleDirectories_expected(String name) throws IOException {
> 107: try (ImageReader reader = ImageReader.open(image, PreviewMode.DISABLED)) {
The `image` variable might be better documented in upper case to make it easier to see its a pre-computed value.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2380657663
PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2383305695
PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2383319595
PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2383427549
More information about the valhalla-dev
mailing list