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

[Roger Riggs]

On Wed, 8 Oct 2025 12:35:21 GMT, David Beaumont <duke at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java line 87:
>> 
>>> 85:      * for {@code /packages/xxx} directories).
>>> 86:      */
>>> 87:     public static final int FLAGS_IS_PREVIEW_ONLY = 0x4;
>> 
>> The preview-only case is expected to be the empty set. There seems to be a lot of code dedicated to it.
>
> Well it's a small set, but vitally important. Preview only is the case where you can't lookup the normal resource first and check its flags to see if there's a preview version. Since you must correct find all preview-only resources, you are left with either:
> 
> * *always* speculatively looking up all resources in their preview location when not found otherwise.
> * Doing something "cleverer" to reduce double lookups.
> 
> In the C++ class-loader (as I understand it) most lookups will be for things referenced in other class files, and thus expected to exist. This makes "if you can't find the normal version, look for the preview version" acceptable (I think) because this should be only in cases where you're looking for something that should exist at one of the two paths.
> 
> In the Java stuff, there's a lot of speculative lookup, especially for non-class resources. All modules are tested for any non-existent resource. So in this case the strategy needs to be cleverer or you'll basically (at least) double the cost of a lot of things.
> 
> So I settled on the easiest approach to be to just (efficiently) pre-scan for preview resources and put them in the node cache during init (this is only about the same work the old code did for link nodes etc.). Once they're cached, the lookup code no longer has to care about 2nd lookups or anything. Preview versions and preview-only versions are just treated the same, they're in the cache, so they're found and returned without looking at anything else.
> 
> This is partly why there's the "ensureCached" method which guarantees you never replace what's in the cache with anything else (so preview stuff put there during init of the reader is "safe" from accidental overwriting).
> 
> The new flag stuff and change of ordering of package entries is there mostly to support efficient pre-scanning of the small set of preview only resources.

I'm still going to say, there are no plans to have any Preview-only resources.

And yes, a preview-only flag would be useful to (mostly) avoid a second lookup.
If I understand the design, there is still a location in the normal tree with the preview-only flag but there is no resource at that location, only the flags.  And a second lookup is needed to find the location in the preview heirarchy. The caching avoids repeated 2nd lookups.

>> src/java.base/share/classes/jdk/internal/jrtfs/JrtFileSystemProvider.java line 125:
>> 
>>> 123:         } else if (envValue instanceof String && Boolean.parseBoolean((String) envValue)) {
>>> 124:             return PreviewMode.ENABLED;
>>> 125:         }
>> 
>> This kind of ambiguous type for the environment variable should not be supported. 
>> Pick a type and stick to it.  
>> Whereever the jrt: file system is specified will need a description for enablePreview to be added to the spec.
>
> Other file-system flags already do this, so I was just copying that approach. Happy to not do this though.
> Which do you suggest, Boolean or String?

Strings are more frequent and conventional.

>> src/java.base/share/classes/jdk/internal/jrtfs/SystemImage.java line 79:
>> 
>>> 77: 
>>> 78:         // TODO: Maybe throw if enablePreview attempted for exploded image?
>>> 79: 
>> 
>> Exploded image is useful for debugging, it could support preview mode using the same META-INF directory structure.
>
> Indeed. I can support it later, I'm just wondering if, in this partially complete state, I should complain or not.
> I've got most of the code for that done somewhere or other.

Throwing with a message would avoid doubt about whether it is supported.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2414290560
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2414296625
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2414298186