On Mon, 26 Jan 2026 12:42:06 GMT, David Beaumont <[email protected]> wrote:
> Implementation of preview-mode support for jimage modules file, migrated from
> Valhalla related work (see JDK-8352750).
>
> This PR (the first of several) migrates work from Valhalla (lworld) to the
> JDK mainline repository in relation to "preview mode" support. It affects the
> creation and reading of the jimage file, both in Java
> (BasicImageReader/ImageReader) and C++ (imageFile.xpp/jimage.xpp).
>
> Preview mode is a mechanism by which alternate version of JDK class files and
> resources can be made available for class loading and reflection when the
> '--enable-preview' flag is passed to the runtime.
>
> Alternate classes/resource appear in each module under the:
>
> /<module>/META-INF/preview/<path-to>/<resource-or-class>
>
> and replace the original:
>
> /<module>/<path-to>/<resource-or-class>
>
> files when preview mode is enabled.
>
> While initially useful for Valhalla work, this mechanism will be used for
> other cases where preview features (in the JEP 12 sense) require alternate
> classes/resources to be provided. None of the changes in this (or the
> follow-up PRs) are Valhalla specific.
>
> In this PR:
> * the writing of jimage files is modified to recognize and handle preview
> mode paths
> * flags in the jimage file are added or modified to support preview mode
> efficiently
> * (C++) the class loader is modified to permit reading preview versions of
> classes
> * (Java) the image reader and associated JRT file-system classes are modified
> to permit reading preview files
> * unit tests are added to ensure preview mode works as expected when enabled
> * (temporary) any code calling into the affected API (other than tests)
> specifies that preview mode is disabled
>
> Future PRs will add the plumbing to enable preview mode correctly, but with
> the PR there should be no observable change in behaviour (especially since no
> preview classes or resources are being supplied at this point).
Looks good.
Please update copyrights.
A second reviewer would be good for this re-implementation.
src/hotspot/share/classfile/classLoader.cpp line 269:
> 267: static JImageFile* jimage_non_null() {
> 268: assert(jimage_exists(), "should have been opened by
> ClassLoader::lookup_vm_options "
> 269: "and remained throughout normal JVM lifetime");
"remained" -> "remain open"
src/hotspot/share/runtime/arguments.cpp line 2995:
> 2993:
> 2994: // Called after ClassLoader::lookup_vm_options() but before class
> loading begins.
> 2995: // TODO: Obtain and pass correct preview mode flag value here.
Suggestion: A hotspot convention is to include the bugid of the fix/enhancement
that is pending.
Since you've planned the progression of PR's that may be useful here. YMMV.
src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 85:
> 83: return new ModuleReference(moduleName, previewFlag(isPreview));
> 84: }
> 85:
A one-line comment may be useful here.
src/java.base/share/native/libjimage/imageFile.cpp line 361:
> 359: if (verify_location(location, path)) {
> 360: *size =
> (jlong)location.get_attribute(ImageLocation::ATTRIBUTE_UNCOMPRESSED);
> 361: return offset;
The previous line is also wrongly-indented.
src/java.base/share/native/libjimage/jimage.cpp line 112:
> 110: size_t module_name_len = strlen(module_name);
> 111: size_t name_len = strlen(name);
> 112: size_t preview_infix_len = strlen(preview_infix);
`sizeof` would make this a constant.
-------------
PR Review: https://git.openjdk.org/jdk/pull/29414#pullrequestreview-3787717603
PR Comment: https://git.openjdk.org/jdk/pull/29414#issuecomment-3891551995
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2795758045
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2795792694
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2795824431
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2799342198
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2799363201
