On Thu, 21 Dec 2023 19:10:59 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
> Please review this change for enabling full module graph even when > -Xbootclasspath/a is specified. The validation of -Xbootclasspath/a is > already being done in `FileMapInfo::validate_boot_class_paths()`. The full > module graph will be disabled if there's a mismatch in -Xbootclasspath/a > between dump time and runtime. > > The changes in ClassLoaders.java is for setting up the append class path for > the boot loader retrieved from the CDS archive. It is required because some > existing tests are using the `getResourceAsStream()` api. Those tests would > fail without the change. > > Passed tiers 1 - 4 testing. Changes looks okay although running -Xbootclasspath/a is an outliner that I wouldn't expect to see at dump time or runtime, the motive here seems to be tests. src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java line 71: > 69: URLClassPath bootAppendUcp = (append != null && !append.isEmpty()) > 70: ? new URLClassPath(append, true) > 71: : null; Would you mind renaming this to bootUcp or bcp? The reason is that this is the boot loader's class path, not the append path. The system property has "append" in the name as it's communicating the value of -Xbootclasspath/a to the library code, this came about when -Xbootclasspath and -Xbootclasspath/p were removed. src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java line 76: > 74: BOOT_LOADER = (BootClassLoader) > archivedClassLoaders.bootLoader(); > 75: setArchivedServicesCatalog(BOOT_LOADER); > 76: BOOT_LOADER.setClassPath(bootAppendUcp); It might be clearer to set this before calling setArchivedServicesCatalog. ------------- PR Review: https://git.openjdk.org/jdk/pull/17178#pullrequestreview-1794149461 PR Review Comment: https://git.openjdk.org/jdk/pull/17178#discussion_r1434844648 PR Review Comment: https://git.openjdk.org/jdk/pull/17178#discussion_r1434845035
