RFR: 8368333: [lworld] Add preview mode to ImageReader and JRT file-system [v7]

[Roger Riggs]

On Tue, 14 Oct 2025 18:28:48 GMT, David Beaumont <duke at openjdk.org> wrote:

>> Java changes for supporting preview mode when preview mode resources (with new location flags) are available.
>> 
>> At the moment, this code will operate on non-preview jimage files (1.0) and act as if no preview resources are available by virtue of the default value for missing attributes and package flags being zero (which matches jimage 1.0).
>> 
>> This should be reviewed on top of https://github.com/openjdk/valhalla/pull/1618
>
> David Beaumont has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
> 
>  - Rolled up changes after rebase.
>    
>    * Removing package root flag based on feedback.
>    * Changing existing package flags during writing to match altered flag values.
>    * Feedback changes, and fixing some comments.
>    * Renaming slightly confusing "testEncoder" method.
>    * Fixing unit tests to use new constructor.
>    * Word smithing flags definitions.
>    * Add workaround until new image writing code is in
>    * Clarifying flag docs for /packages/xxx case
>    * Java ImageReader changes for preview mode
>  - Merge branch 'jdk_8366093_cpp/squashed' into jdk_8368333_java/squashed
>  - [[RESET BRANCH FOR MERGE]]
>  - Removing package root flag based on feedback.
>  - Changing existing package flags during writing to match altered flag values.
>  - Feedback changes, and fixing some comments.
>  - Test fixes and feedback changes.
>    
>    * Renaming slightly confusing "testEncoder" method.
>    * Fixing unit tests to use new constructor.
>  - Manually deleting ImageReaderFactory (it returned somehow)
>  - Word smithing flags definitions.
>  - Add workaround until new image writing code is in
>  - ... and 2 more: https://git.openjdk.org/valhalla/compare/60b6ed62...9bbc26c1

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

> 58:     public static final int MAJOR_VERSION = 1;
> 59:     public static final int MINOR_VERSION = 1;
> 60:     private static final int HEADER_SLOTS = 7;

Please add a comment before these constants connecting them to imageFile.hpp.

src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java line 48:

> 46:     public static final int ATTRIBUTE_COMPRESSED = 6;
> 47:     public static final int ATTRIBUTE_UNCOMPRESSED = 7;
> 48:     public static final int ATTRIBUTE_PREVIEW_FLAGS = 8;

Please add a comment above these constants that the values are defined in ImageFile.hpp.

src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 282:

> 280:         // preview-only nodes. This is used to add preview-only content to
> 281:         // directories as they are completed.
> 282:         private final HashMap<String, Directory> previewDirectoriesToMerge;

Can this be cleared or freed after the ImageReader is open. Its no longer needed.

src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 525:

> 523:             }
> 524:             ImageLocation loc = findLocation(moduleName, resourcePath);
> 525:             return loc != null && loc.getType() == RESOURCE;

Would there be any benefit to caching the resource here, since it has been found and will likely be opened again.

src/java.base/share/classes/jdk/internal/jimage/ImageStrings.java line 38:

> 36:     // String offset constants are useful for efficient classification
> 37:     // of location entries without string comparison. These may change
> 38:     // between jimage versions (they are checked during initialization).

The checking seem only to be done when ImageStringsWriter is loaded/initialized.
There may be a small risk that a mismatch of the image with the ImageStringsReader could occur.
Can they be checked also by the reader?

src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 57:

> 55:     /** If set, the associated module has resources (in normal or preview mode). */
> 56:     // TODO: Make this private again when image writer code is updated.
> 57:     public static final int FLAGS_HAS_CONTENT = 0x4;

Please change the prefix of these to distinguish them from the ImageLocation flags.
For example, "PKG_" or "MR_".

src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 120:

> 118:      * under many modules, it only has content in one.
> 119:      */
> 120:     public boolean hasContent() {

As suggested, `hasResoruces` would be consistent with the lookups for resources.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449397419
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449382931
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449520220
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449546445
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449424904
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449391627
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2449493953