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

[Yumin Qi]

On Wed, 12 May 2021 15:41:36 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>>> Hi Yumin,
>>> 
>>> I applied your patch and ran ParallelLambdaLoadTest.java for about 400 iterations and I could no longer reproduce the crash in [JDK-8266765](https://bugs.openjdk.java.net/browse/JDK-8266765).
>>> 
>>> As I mentioned in some of the individual comments, we need to properly handle the exceptions that can happen inside `MetaspaceShared::link_and_cleanup_shared_classes`. I think we should change these places:
>>> 
>>> ```
>>> JVM_ENTRY_NO_ENV(void, JVM_BeforeHalt())
>>>   // Link all classes for dynamic CDS dumping before vm exit.
>>>   if (DynamicDumpSharedSpaces) {
>>> -  MetaspaceShared::link_and_cleanup_shared_classes(THREAD);
>>> +  DynamicArchive::prepare_for_dynamic_dumping_at_exit();
>>>   }
>>> ...
>>> 
>>> void JavaThread::invoke_shutdown_hooks() {
>>> ...
>>>   if (DynamicDumpSharedSpaces) {
>>> -  MetaspaceShared::link_and_cleanup_shared_classes(this);
>>> +  DynamicArchive::prepare_for_dynamic_dumping_at_exit();
>>>   }
>>> ```
>>> 
>>> And add a new function like this to catch the exception. If `link_and_cleanup_shared_classes` has thrown an exception, we should not dump the dynamic archive anymore. (We are already do that with the static archive).
>>> 
>>> ```
>>> void DynamicArchive::prepare_for_dynamic_dumping_at_exit() {
>>>   EXCEPTION_MARK;
>>>   ResourceMark rm(THREAD);
>>>   MetaspaceShared::link_and_cleanup_shared_classes(THREAD);
>>>   if (HAS_PENDING_EXCEPTION) {
>>>     log_error(cds)("ArchiveClassesAtExit has failed");
>>>     log_error(cds)("%s: %s", PENDING_EXCEPTION->klass()->external_name(),
>>>                    java_lang_String::as_utf8_string(java_lang_Throwable::message(PENDING_EXCEPTION)));
>>>     // We cannot continue to dump the archive anymore.
>>>     DynamicDumpSharedSpaces = false;
>>>     CLEAR_PENDING_EXCEPTION;
>>>   }
>>> }
>> 
>> The function name should be prepare_for_dynamic_dumping since it is called when jcmd does dynamic dumping which does not exit after dumping archive.
>>> 
>>> I would also suggest writing a new test case for the exception handling: run a program with something like `-Xmx64m -XX:ArchiveClassesAtExit`. The program should completely fill up the Java heap and exit. The dynamic archive should fail to dump due to OOM.
>> 
>> Sure will add test for this.
>
>> The function name should be prepare_for_dynamic_dumping since it is called when jcmd does dynamic dumping which does not exit after dumping archive.
> 
> This function should not be called by the jcmd code. This function catches exceptions that happens inside `MetaspaceShared::link_and_cleanup_shared_classes()`, but in your existing code, jcmd doesn't call `MetaspaceShared::link_and_cleanup_shared_classes()`.

@iklam  thanks for review again.

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

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