On Thu, 12 Mar 2026 14:27:33 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).
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> More feedback tweaks
I've taken an initial pass through the hotspot code and have a few minor
comments. Overall the API changes and refactoring seems fine. I don't
understand the overall big picture here however.
src/hotspot/share/classfile/classLoader.cpp line 277:
> 275: // jimage_is_preview_enabled() can be called to correctly determine the
> access mode.
> 276: static bool jimage_is_initialized() {
> 277: return jimage_exists() && Preview_mode != PREVIEW_MODE_UNINITIALIZED;
If the JImage doesn't exist then presumably the Preview_mode will never change
from PREVIEW_MODE_UNINITIALIZED, so that is all we need to check here.
src/hotspot/share/classfile/classLoader.cpp line 291:
> 289: const char* file_name,
> 290: bool is_preview,
> 291: jlong *size) {
Suggestion:
jlong* size) {
src/hotspot/share/classfile/classLoader.cpp line 1463:
> 1461: jimage_find_resource(JAVA_BASE_NAME,
> "jdk/internal/vm/options", /* is_preview */ false, &size);
> 1462: if (location != 0) {
> 1463: char *options = NEW_C_HEAP_ARRAY(char, size+1, mtClass);
Suggestion:
char* options = NEW_C_HEAP_ARRAY(char, size+1, mtClass);
src/hotspot/share/classfile/classLoader.cpp line 1474:
> 1472: // Finishes initializing the JImageFile (if present) by setting the
> access mode.
> 1473: void ClassLoader::set_preview_mode(bool enable_preview) {
> 1474: assert(Preview_mode == PREVIEW_MODE_UNINITIALIZED, "set_preview_mode
> must not be called twice");
So should this check, or assert, that the JImageFile exists?
src/hotspot/share/classfile/classLoader.hpp line 364:
> 362: static char* lookup_vm_options();
> 363:
> 364: // Called once, after all flags are processed, to finish initializing
> the
This method doesn't "finish initializing the JImage file" it just marks it as
initialized.
-------------
PR Review: https://git.openjdk.org/jdk/pull/29414#pullrequestreview-3941897326
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2929291313
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2929292888
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2929307749
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2929312184
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r2929325034
