RFR: 8265602: -XX:DumpLoadedClassList should support custom loaders [v2]

Ioi Lam iklam at openjdk.java.net
Wed Aug 4 06:40:34 UTC 2021


On Wed, 4 Aug 2021 01:40:18 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @calvinccheung comments
>
> src/hotspot/share/cds/archiveUtils.cpp line 336:
> 
>> 334:   if (ClassListWriter::is_enabled()) {
>> 335:     if (SystemDictionaryShared::is_supported_invokedynamic(bootstrap_specifier)) {
>> 336:       ResourceMark rm(THREAD);
> 
> Line 336 could be moved after line 338.

Fixed.

> src/hotspot/share/cds/classListWriter.cpp line 54:
> 
>> 52: 
>> 53: void ClassListWriter::write(const InstanceKlass* k, const ClassFileStream* cfs) {
>> 54:   assert(is_enabled(), "must be");
> 
> Should `assert_locked()` be added here?

This function doesn't need the lock, since it doesn't directly use `ClassListWriter::_classlist_file` or `ClassListWriter::_id_table`, which are protected by `ClassListFile_lock`. The lock will be taken a few lines down, in the `ClassListWriter` constructor.

Your comment reminded me to look for places that need the lock. As a result, I added `assert_locked()` to `ClassListWriter::write_to_stream()`.

> src/hotspot/share/cds/classListWriter.hpp line 63:
> 
>> 61:     return false;
>> 62:   }
>> 63: #endif
> 
> Please add `// INCLUDE_CDS` after `endif`.

Fixed.

> test/hotspot/jtreg/runtime/cds/appcds/customLoader/test-classes/CustomLoadee4WithLambda.java line 30:
> 
>> 28:                 System.out.println("Hello inside a Lambda expression");
>> 29:             });
>> 30:     }
> 
> We have a similar class in dynamicArchive/test-classes/LambHello.java.
> To avoid changes in existing tests, maybe you can just add a `test()` method there.

`LambHello` is in a separate directory and is used by several existing tests that require the function `doTest()`. If I add another `test()` function there it kind of looks odd. Also, I don't want to introduce inter-dependencies between too many tests. Since this class is small, I prefer to keep it separate.

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

PR: https://git.openjdk.java.net/jdk/pull/4952


More information about the hotspot-runtime-dev mailing list