RFR: 8295893: Improve printing of Constant Pool Cache Entries [v6]

Tue Nov 1 21:08:26 UTC 2022

On Tue, 1 Nov 2022 18:35:42 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> As an extension of [JDK-8292699](https://bugs.openjdk.org/browse/JDK-8292699), this aims to further improve the printing of Constant Pool Cache entries. The contents and flag are decoded into human readable text with an appendix printed as before.
>> 
>> The text format and contents are tentative, please review.
>> 
>> Here is an example output when using `findmethod()`:
>> 
>> "Executing findmethod"
>> flags (bitmask):
>>    0x01  - print names of methods
>>    0x02  - print bytecodes
>>    0x04  - print the address of bytecodes
>>    0x08  - print info for invokedynamic
>>    0x10  - print info for invokehandle
>> 
>> [  0] 0x0000000801000800 class Concat0 loader data: 0x00007ffff02ddeb0 for instance a 'jdk/internal/loader/ClassLoaders$AppClassLoader'{0x00000007fef59110}
>> 0x00007fffa0400368 static method main : ([Ljava/lang/String;)V
>>    0 iconst_0
>>    1 istore_1
>>    2 iload_1
>>    3 iconst_2
>>    4 if_icmpge 24
>>    7 getstatic 7 <Concat0.s/Ljava/lang/String;> 
>>   10 invokedynamic bsm=31 13 <makeConcatWithConstants(Ljava/lang/String;)Ljava/lang/String;>
>>   BSM: REF_invokeStatic 32 <java/lang/invoke/StringConcatFactory.makeConcatWithConstants(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;> 
>>   arguments[1] = {
>>      000
>>   }
>>   ConstantPoolCacheEntry:   4
>>  - this: 0x00007fffa0400570
>>  - bytecode 1: invokedynamic ba
>>  - bytecode 2: nop 00
>>  - cp index:    13
>>  - F1:  [   0x00000008000c8658]
>>  - F2:  [   0x0000000000000003]
>>  -     Method: 0x00000008000c8658 java.lang.Object java.lang.invoke.Invokers$Holder.linkToTargetMethod(java.lang.Object, java.lang.Object)
>>  - flag values: [08|0|0|1|1|0|1|0|0|0|00|00|02]
>>  - tos: object
>>  - local signature: 1
>>  - has appendix: 1
>>  - forced virtual: 0
>>  - final: 1
>>  - virtual Final: 0
>>  - resolution Failed: 0
>>  - num Parameters: 02
>>   Method: 0x00000008000c8658 java/lang/invoke/Invokers$Holder.linkToTargetMethod(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
>>   appendix: java.lang.invoke.BoundMethodHandle$Species_LL 
>> {0x000000011f021360} - klass: 'java/lang/invoke/BoundMethodHandle$Species_LL'
>>  - ---- fields (total size 5 words):
>>  - private 'customizationCount' 'B' @12  0 (0x00)
>>  - private volatile 'updateInProgress' 'Z' @13  false (0x00)
>>  - private final 'type' 'Ljava/lang/invoke/MethodType;' @16  a 'java/lang/invoke/MethodType'{0x000000011f0185b0} = (Ljava/lang/String;)Ljava/lang/String; (0x23e030b6)
>>  - final 'form' 'Ljava/lang/invoke/LambdaForm;' @20  a 'java/lang/invoke/LambdaForm'{0x000000011f01df40} => a 'java/lang/invoke/MemberName'{0x000000011f0211e8} = {method} {0x00007fffa04012a8} 'invoke' '(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;' in 'java/lang/invoke/LambdaForm$MH+0x0000000801000400' (0x23e03be8)
>>  - private 'asTypeCache' 'Ljava/lang/invoke/MethodHandle;' @24  NULL (0x00000000)
>>  - private 'asTypeSoftCache' 'Ljava/lang/ref/SoftReference;' @28  NULL (0x00000000)
>>  - final 'argL0' 'Ljava/lang/Object;' @32  a 'java/lang/invoke/DirectMethodHandle'{0x000000011f019b70} (0x23e0336e)
>>  - final 'argL1' 'Ljava/lang/Object;' @36  "000"{0x000000011f0193d0} (0x23e0327a)
>>                  -------------
>>   15 putstatic 17 <Concat0.d/Ljava/lang/String;> 
>>   18 iinc #1 1
>>   21 goto 2
>>   24 return
>
> Matias Saavedra Silva has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   Added gtest

Changes requested by iklam (Reviewer).

src/hotspot/share/oops/cpCache.cpp line 639:

> 637:     st->print_cr(" - F2:  [   " PTR_FORMAT "]", (intptr_t)_f2);
> 638:     st->print_cr(" -     Method: " INTPTR_FORMAT " %s", p2i(m), m != nullptr ? m->external_name() : nullptr);
> 639:     st->print_cr(" - flag values: [%02x|0|0|%01x|%01x|%01x|%01x|0|%01x|%01x|00|00|%02x]",

To be consistent with the other output, `method` should be lower case and should be preceded by a single space.

src/hotspot/share/oops/cpCache.cpp line 643:

> 641:                is_forced_virtual(), is_final(), is_vfinal(),
> 642:                indy_resolution_failed(), parameter_size());
> 643:     st->print_cr(" - tos: %s\n - local signature: %01x\n"

The parameters should be aligned after the open parenthesis. Please fix the other cases as well. You should keep the formatting consistent with the rest of the file.

st->print_cr(" - flag values: [%02x|0|0|%01x|%01x|%01x|%01x|0|%01x|%01x|00|00|%02x]",
             flag_state(), has_local_signature(), has_appendix(),
             is_forced_virtual(), is_final(), is_vfinal(),
             indy_resolution_failed(), parameter_size());

src/hotspot/share/oops/cpCache.cpp line 651:

> 649:                indy_resolution_failed(), parameter_size());
> 650:     if ((bytecode_1() == Bytecodes::_invokehandle ||
> 651:        bytecode_1() == Bytecodes::_invokedynamic)) {

Should be:

    if (bytecode_1() == Bytecodes::_invokehandle ||
        bytecode_1() == Bytecodes::_invokedynamic) {

src/hotspot/share/oops/cpCache.cpp line 654:

> 652:       oop appendix = appendix_if_resolved(cph);
> 653:       if (m != nullptr) {
> 654:         st->print_cr("  Method%s: " INTPTR_FORMAT " %s.%s%s",

The method is already printed above, so there's no need to print it again. The only difference between here and the above is the "(native)" part, which is unimportant and can be removed.

test/hotspot/gtest/oops/test_cpCache_output.cpp line 3:

> 1: /*
> 2:  * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.
> 3:  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

test/hotspot/gtest/oops/test_cpCache_output.cpp line 66:

> 64:   ASSERT_TRUE(strstr(output, "volatile:") != NULL) << "must have volatile flag";
> 65:   ASSERT_TRUE(strstr(output, "field index:") != NULL) << "must have field index";
> 66: }

Need to add a newline at the end of the file.

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

PR: https://git.openjdk.org/jdk/pull/10860