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 04:40:30 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.

> src/hotspot/share/cds/dynamicArchive.cpp line 363:
> 
>> 361: }
>> 362: 
>> 363: void DynamicArchive::dump(TRAPS) {
> 
> TRAPS is no longer needed since this function doesn't throw exceptions anymore.

Will revert to original version.

> src/hotspot/share/cds/metaspaceShared.cpp line 638:
> 
>> 636:   // Collect all loaded ClassLoaderData.
>> 637:   ResourceMark rm;
>> 638:   regenerate_lambdaforminvokers_holders(THREAD);
> 
> Should be `CHECK`.

will remove the call from this spot, move to its original spot to call regeneration.

> src/hotspot/share/prims/jvm.cpp line 3689:
> 
>> 3687:   Handle file_handle(THREAD, JNIHandles::resolve_non_null(archiveName));
>> 3688:   char* archive_name  = java_lang_String::as_utf8_string(file_handle());
>> 3689:   MetaspaceShared::regenerate_lambdaforminvokers_holders(THREAD);
> 
> The `THREAD` should be `CHECK`.

Sure.

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

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