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

[David Beaumont]

On Tue, 21 Oct 2025 19:43:04 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> 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/802d18fe...9bbc26c1
>
> 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.

It's used when directories are completed (as stated in the comment):


    List<Node> previewOnlyNodes = getPreviewNodesToMerge(dir);
    ...
    children.addAll(previewOnlyNodes);

> 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?

Hmm, I don't see a nice way to do this at runtime since you can't lookup string offsets by their value (as far as I can see).

It's no more fragile than the old code (which had hard-coded constants for "class" and "" (and wasn't self-checking their validity). At least now it's tested at build time.

If the version number matches it should be safe. I'll add a comment about how changing existing entries is problematic.

> 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.

Will do.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2453165416
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2453158966
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2453160430