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

Yumin Qi minqi at openjdk.java.net
Fri Apr 30 03:01:06 UTC 2021


On Thu, 29 Apr 2021 23:48:37 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> 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/c3c9da7e...0846f121
>
> 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?");
> }

Do you mean DumpSharedSpaces? So the message will be different between static (DumpSharedSpaces) and dynamic, right?

> 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`.

OK, I was think of that, if a function or macro and finally took macro.

> 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.

Here is the original code, and it is necessary, KlassFactory::create_from_stream could throw exception.

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

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


More information about the hotspot-dev mailing list