RFR: 8266764: [REDO] JDK-8255493 Support for pre-generated java.lang.invoke classes in CDS dynamic archive [v6]

Calvin Cheung ccheung at openjdk.java.net
Fri May 14 17:35:42 UTC 2021


On Fri, 14 May 2021 16:06:02 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Hi, please review
>>   This is REDO for jdk-8255493, the integration of the fix caused ParallelLambdaLoadTest.java failed. 
>>   The old PR description:
>> ------------- old ------------
>> When do dynamic dump, the pre-generated lambda form classes from java.lang.invoke are not stored in dynamic archive. The patch will regenerate the four holder classes,
>> "java.lang.invoke.Invokers$Holder",
>> "java.lang.invoke.DirectMethodHandle$Holder",
>> "java.lang.invoke.DelegatingMethodHandle$Holder",
>> "java.lang.invoke.LambdaForm$Holder"
>> which will include the versions in static archive and new loaded functions all together and stored in dynamic archive. New test case added.
>> ------------ new -------------
>> The new fix  (incremental)
>> 1)  Added a lock to protect LambdaFormInvokers::_lambdaform_lines, at dynamic dump case, there are potentially concurrent issue the list is added from multiple threads at same time  regeneration of holder class is reading from the list.
>> 2) Added a new function to MetaspaceShared, MetaspaceShared::regenerate_lambdaforminvokers_holders which calls into LambdaFormInvokers::LambdaFormInvokers::regenerate_holder_classes, and handle exceptions, this way even the regeneration fails, no affect to normal dump process.
>> 3) Move the call to regenerate holder classes to MetaspaceShared::link_and_cleanup_shared_classes, this is before java shutdown hook execution, since after that, it is not safe to call into java. 
>> 4) Since change in 3), jcmd to dynamic dump, we need call MetaspaceShared::regenerate_lambdaforminvokers_holders before do dumping.
>> 
>> Tests: tier1,tier2,tier3,tier4
>>            local test on runtime/cds
>>            local test ParallelLambdaLoadTest.java in loops without crash.
>> 
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
> 
>  - Merge with master, resolve conflict
>  - New added DynamicArchive::prepare_for_dynamic_dumping_at_exit should be CDS only
>  - Fix whitespace
>  - Added DynamicArchive::prepare_for_dynamic_dumping_at_exit to replace MetaspaceShared::regenerate_lambdaforminvokers_holders, the latter was removed. Added test case"
>  - Remove unused variables
>  - 8266764: [REDO] JDK-8255493 Support for pre-generated java.lang.invoke classes in CDS dynamic archive

Looks good. One small nit in one of the tests.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestDynamicDumpAtOom.java line 57:

> 55:              "1024").assertAbnormalExit(output -> {
> 56:                  output.shouldContain("ArchiveClassesAtExit has failed")
> 57:                         .shouldContain("java.lang.OutOfMemoryError: Java heap space");

One extra blank space before `.shouldContain` at line 57.

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

Marked as reviewed by ccheung (Reviewer).

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


More information about the hotspot-runtime-dev mailing list