RFR: 8280682: Refactor AOT code source validation checks [v5]

David Holmes dholmes at openjdk.org
Mon Feb 24 00:07:02 UTC 2025


On Fri, 21 Feb 2025 06:19:55 GMT, Calvin Cheung <ccheung at 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


More information about the serviceability-dev mailing list