RFR: 8329706: Implement -XX:+AOTClassLinking [v9]

[Ioi Lam]

On Wed, 18 Sep 2024 05:01:00 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixed ZERO build
>
> src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 66:
> 
>> 64: 
>> 65: void AOTLinkedClassBulkLoader::load_classes_in_loader(JavaThread* current, AOTLinkedClassCategory class_category, oop class_loader_oop) {
>> 66:   ExceptionMark em(current);
> 
> Why do you need the EM when you are explicitly checking for exceptions?

Removed.

> src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 67:
> 
>> 65: void AOTLinkedClassBulkLoader::load_classes_in_loader(JavaThread* current, AOTLinkedClassCategory class_category, oop class_loader_oop) {
>> 66:   ExceptionMark em(current);
>> 67:   ResourceMark rm(current);
> 
> The RM should go where it is actually needed for the logging.

Fixed

> src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 68:
> 
>> 66:   ExceptionMark em(current);
>> 67:   ResourceMark rm(current);
>> 68:   HandleMark hm(current);
> 
> Why do you need a HM here?

It was needed when this code was called from a different path. It's no longer needed now, so I removed it.

> src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 95:
> 
>> 93: 
>> 94:   if (Universe::is_fully_initialized() && VerifyDuringStartup) {
>> 95:     // Make sure we're still in a clean slate.
> 
> Suggestion:
> 
>     // Make sure we're still in a clean state.

Fixed

> src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 132:
> 
>> 130:     break;
>> 131:   case AOTLinkedClassCategory::UNREGISTERED:
>> 132:     ShouldNotReachHere(); // Currently aot-linked classes are not supported for this category.
> 
> Suggestion:
> 
>   case AOTLinkedClassCategory::UNREGISTERED:
>   default:
>     ShouldNotReachHere(); // Currently aot-linked classes are not supported for this category.

Fixed.

> src/hotspot/share/cds/aotLinkedClassTable.hpp line 34:
> 
>> 32: class SerializeClosure;
>> 33: 
>> 34: // Classes to be buik-loaded, in the "linked" state, at VM bootstrap.
> 
> Suggestion:
> 
> // Classes to be bulk-loaded, in the "linked" state, at VM bootstrap.

Fixed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766121555
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766121383
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766121348
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766121642
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766121689
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766121855