On Wed, 5 Feb 2025 22:32:58 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
> This changset refactors CDS class paths and module paths validation code into > a new class `AOTCodeSource` and related class `AOTCodeSourceConfig`. Code has > been moved from filemap.[c|h]pp, classLoader.[c|h]pp, and > classLoaderExt.[c|h]pp to aotCodeSource.[c|h]pp. CDS dependencies have been > removed from `classLoader.cpp`. More refactoring could be done, such as > removing `classLoaderExt.cpp`, in a future RFE. > > Passed tiers 1 - 5 testing. Looks good. A few suggestions for the in-line comments. src/hotspot/share/cds/aotCodeSource.cpp line 133: > 131: > 132: // AllCodeSourceStreams is used to iterate over all the code sources that > 133: // are available to the application from -Xbootclasspath, -classpath and > --module-path Consider adding this comment: // When creating an AOT cache, we store the contents from AllCodeSourceStreams // into an array of AOTCodeSources. See AOTCodeSourceConfig::dumptime_init_helper(). // When loading the AOT cache in a production run, we compare the contents of the // stored AOTCodeSources against the current AllCodeSourceStreams to determine whether // the AOT cache is compatible with the current JVM. See AOTCodeSourceConfig::validate(). src/hotspot/share/cds/aotCodeSource.hpp line 126: > 124: // Non-existent entries are recored during AOTCache creation. Those > non-existent entries > 125: // must not exist during runtime. > 126: // Typos: - "subjected to AOTCodeSourceConfig::validate()" -- the function has two parameters, but we can omit them in this comment - "validation is performed on *the* AOTCodeSources" - "during AOTCache creation *are* the same" - "on-existent entries are *recorded*" ------------- PR Review: https://git.openjdk.org/jdk/pull/23476#pullrequestreview-2607702313 PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1950250159 PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1950244507
