On Tue, 23 Sep 2025 19:25:22 GMT, David Beaumont <[email protected]> wrote:
> Adds support for writing preview related flags into jimage files. > > Preview mode is complex. It's not nearly as simple as "does something in > /modules/xxx/... have an entry in /modules/xxx/META-INF/preview/...". > > Specific issues include: > > Supporting preview-only resources without forcing a double lookup on > everything. > Changing the set of entries in /packages/xxx directories to account for > preview only packages in some modules. > Minimising the work done during image reader initialization to only need to > process the small number of preview resources (rather than scanning the whole > file to look for them). > The new flags added by this code address these issues, but calculating them > correctly with only minor adjustments to the existing code was not feasible, > it just became a lot larger and very complex. > > To address this, a new type (ModuleReference) is introduced to track and then > merge information about packages seen in each module. This allows a much > simpler inner loop for processing resource paths when building the node tree, > combined with a subsequent merging stage to produce the final package > information for each module. > > Not that since ModuleReference is needed during jimage reading, that class is > already present in the previous PR on which this is based, but it starts to > be used to calculate the module flags in this PR. > > This PR can also adds the ImageReader unit tests for preview mode, which rely > on being able to generate jimage files with preview mode flags in. > > Compare and review this against https://github.com/openjdk/valhalla/pull/1619. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java line 290: > 288: } > 289: > 290: private static ResourcePool getResourcePool( Pulled out so the variable holding it is effectively final and can be used in a lambda expression. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageLocationWriter.java line 93: > 91: > 92: return new ImageLocationWriter(strings) > 93: .addAttribute(ATTRIBUTE_MODULE, moduleName) This got reformatted correctly to 8 indent because of the new entry. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 151: > 149: * occur (e.g. {@code HAS_NORMAL_CONTENT + IS_PREVIEW_ONLY}). > 150: * > 151: * <p>Package node entries are sorted by name, with the exception > that (if This is out of date (along with the test). It's now sorted "preview first" and then "by name". The reason for the change is that I had made an incorrect assumption that I didn't need to process empty packages from preview resources, but I do (they still appear in /packages/xxx directories). src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 252: > 250: packageToModules.forEach((pkgName, modRefs) -> { > 251: // Merge multiple refs for the same module. > 252: List<ModuleReference> pkgModules = modRefs.stream() This is where module references are merged so each package has the right set of flags according to all the resources visited in the module (empty, non-empty, preview, preview-only...). This replaces the use of the two maps (moduleToPackage and packageToModule) and a bunch of other logic. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 298: > 296: ModuleReference emptyRef = > ModuleReference.forEmptyPackage(modName, isPreviewPath); > 297: > 298: // Work down through empty packages to final resource. This loop and the section after replaces the logic between L191 and L238 in the original code. Instead of two maps (which would be 4 if we tried to add preview support that way) with near-duplicated logic, there's now just "walk down the path creating empty package nodes until the last segment is reached, then exit and create the resource node in a non-empty package". The empty/non-empty/preview nature of these packages is all handled when the module references are merged later, but for now they are independent. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 431: > 429: ByteBuffer byteBuffer = ByteBuffer.allocate(8 * > refs.size()); > 430: byteBuffer.order(writer.getByteOrder()); > 431: ModuleReference.write(refs, byteBuffer.asIntBuffer(), > writer::addString); Call into the ModuleReference code added in the previous PR to write the package flags and offsets. test/jdk/tools/jlink/whitebox/jdk.jlink/jdk/tools/jlink/internal/ImageResourcesTreeTest.java line 197: > 195: } > 196: > 197: @Test This test needs updating. The order of the entries has changed. Sorry for not spotting sooner. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373352347 PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373354187 PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373387987 PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373377751 PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373371691 PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373380118 PR Review Comment: https://git.openjdk.org/valhalla/pull/1621#discussion_r2373357299
