On Fri, 21 Feb 2025 06:19:55 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.
>
> Calvin Cheung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> rename classes and add vm_exit_during_initialization call
A couple of minor suggestions, but otherwise nothing further from me.
Thanks
src/hotspot/share/cds/aotClassLocation.cpp line 53:
> 51: const AOTClassLocationConfig* AOTClassLocationConfig::_runtime_instance =
> nullptr;
> 52:
> 53: // A ClassLocationStream represents a list of code sources, which can be
> iterated using
Suggestion:
// A ClassLocationStream represents a list of code locations, which can be
iterated using
src/hotspot/share/cds/aotClassLocation.cpp line 133:
> 131: };
> 132:
> 133: // AllClassLocationStreams is used to iterate over all the code sources
> that
Suggestion:
// AllClassLocationStreams is used to iterate over all the code locations that
src/hotspot/share/cds/aotClassLocation.hpp line 122:
> 120: // AOTClassLocations (subjected to AOTClassLocationConfig::validate()).
> 121: //
> 122: // In general, validation is performed on the AOTClassLocations to
> ensure the code sources used
Suggestion:
// In general, validation is performed on the AOTClassLocations to ensure the
code locations used
src/hotspot/share/classfile/classLoaderDataShared.cpp line 157:
> 155: }
> 156:
> 157: void ClassLoaderDataShared::ensure_module_entry_table_exist(oop
> class_loader) {
Suggestion:
void ClassLoaderDataShared::ensure_module_entry_table_exists(oop class_loader) {
Tables exist, but a single table exists.
src/hotspot/share/classfile/classLoaderDataShared.hpp line 37:
> 35: class ClassLoaderDataShared : AllStatic {
> 36: static bool _full_module_graph_loaded;
> 37: static void ensure_module_entry_table_exist(oop class_loader);
Suggestion:
static void ensure_module_entry_table_exists(oop class_loader);
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/23476#pullrequestreview-2635829484
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1966936547
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1966936586
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1966936262
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1966939280
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1966939500
