RFR: 8255493: Support for pre-generated java.lang.invoke classes in CDS dynamic archive [v11]

[Ioi Lam]

On Thu, 29 Apr 2021 22:30:40 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Hi, Please review
>> 
>>   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.
>>   (Minor change to PrintSharedArchiveAtExit, which the index is not consecutive)
>> 
>>   Tests: tier1,tier2,tier3,tier4
>>   
>>   Thanks
>>   Yumin
>
> Yumin Qi has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 16 additional commits since the last revision:
> 
>  - Changed DynamicArchive::dump as static counterpart. Suppressed exceptions in LambdaFormInvokers::regenerate_holder_classes except for oom
>  - Merge branch 'master' into jdk-8255493
>  - Merge branch 'master' of ssh://github.com/yminqi/jdk into jdk-8255493
>  - Only store four lambda invoker (which will be regenerated in dynamic dump) lines in static archive
>  - Removed TRAP from regenerate_holder_classes, also correct SharedLambdaDictionaryPrinter index
>  - Merge branch 'master' into jdk-8255493
>  - Restore filemap.[ch]pp
>  - Remove unnecessary file header including
>  - Removed unused function static_archive_invokers(), add back accidentally deleted empty line
>  - Changed DEBUG_ONLY to log_debug, make small changes for format, added bug tag to test
>  - ... and 6 more: https://git.openjdk.java.net/jdk/compare/65d4ba4a...0846f121

Changes requested by iklam (Reviewer).

src/hotspot/share/cds/lambdaFormInvokers.cpp line 84:

> 82: #define HANDLE_IF_HAS_EXCEPTION                                                                         \
> 83:   if (HAS_PENDING_EXCEPTION) {                                                                          \
> 84:     if (!PENDING_EXCEPTION->is_a(vmClasses::OutOfMemoryError_klass())) {                                \

I would suggest adding a comment and error message to explain why we ignore the error:


// We may get an exception if the classlist contains an error (or an outdated entry generated by
// an older JDK).
if (DumpSharedArchive) {
  log_error(cds)("Failed to generate LambdaForm holder classes. Is your classlist out of date?");
} else {
  log_error(cds)("Failed to generate LambdaForm holder classes. Was the base archive generated with an outdated classlist?");
}

src/hotspot/share/cds/lambdaFormInvokers.cpp line 123:

> 121:   JavaCalls::call_static(&result, cds_klass, method, signrs, list_lines, THREAD);
> 122: 
> 123:   HANDLE_IF_HAS_EXCEPTION;

Instead of using a macro, I think the code can be more readable like this:


if (check_exception(THREAD)) {
    return;
}


and `check_exception` can contain the logic of `HANDLE_IF_HAS_EXCEPTION`.

src/hotspot/share/cds/lambdaFormInvokers.cpp line 141:

> 139:     ClassFileStream st((u1*)buf, len, NULL, ClassFileStream::verify);
> 140:     reload_class(class_name, st, THREAD);
> 141:     HANDLE_IF_HAS_EXCEPTION;

Is this necessary? I think bad classlists will cause errors in the JavaCalls::call_static call above, but not here.

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

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