On Tue, 24 Mar 2026 12:04:18 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 with a new target base due to a > merge or a rebase. The pull request now contains 12 commits: > > - Merge branch 'master' into jimage_preview_mode > - rename jimage_exists to jimage_is_open > - Feedback tweaks > - Feedback tweaks > - More feedback tweaks > - Updated copyright > - Feedback changes > - Merge branch 'master' into jimage_preview_mode > - Merge branch 'master' into jimage_preview_mode > - undo exploded image changes for now > - ... and 2 more: https://git.openjdk.org/jdk/compare/7695b1f9...0e802079 src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 275: > 273: // preview-only nodes. This is used to add preview-only content > to > 274: // directories as they are completed. > 275: private final Map<String, Directory> previewDirectoriesToMerge; I think it would be better if we could use the stick with the term "overlay" when speaking of the resources in /META-INF/preview as it is confusing to see "merge" in some of the method names and comments. Also would it be possible to explain what you mean by "relativized mapping" here? src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 37: > 35: > 36: /** > 37: * Represents the module entries stored in the buffer of {@code > "/packages/xxx"} Can we rename this to something like a ModuleRef (to avoid confusion with j.l.module.ModuleReference at the use-sites) and improve the class description to make it clear why it is needed. Right now it says it references "module entries" but it's more of a way to easily check if a module has resources or what its flags are. I'm also a bit puzzled by the static method readNameOffsets as it feels out of place, doesn't use ModuleReference. src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 30: > 28: > 29: /** > 30: * Accesses the underlying resource entries in a jimage file. This interface is confusing as it provides a way to enumerate the resources in a module, and also get the content/size of resources by "jimage name" that seems to be independent of module name. It seems reasonable to introduce an interface for accessing the resources in the jimage ("raw" API as used in some comments). That would have the getSize and getSize methods. A method that enumerates the resources in a module sits above this. src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 33: > 31: * > 32: * <p>This API is designed only for use by the jlink classes, which read > the raw > 33: * jimage files. Use the {@link ImageReader} API to read jimage contents > at "read the raw jimage files" is confusing as there is only one jimage file that the jlink tool generates. Is the comment about the "jimage" tool rather than "jlink" tool? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3005995993 PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3006010131 PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3006037357 PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3006037993
