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