8295893: Improve printing of Constant Pool Cache Entries #10860

matias9927 · 2022-10-25T19:37:12Z

As an extension of 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

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8295893: Improve printing of Constant Pool Cache Entries

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Coleen Phillimore (@coleenp - Reviewer) ⚠️ Review applies to 452ef598
Ioi Lam (@iklam - Reviewer) ⚠️ Review applies to 05495a94

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10860/head:pull/10860
$ git checkout pull/10860

Update a local copy of the PR:
$ git checkout pull/10860
$ git pull https://git.openjdk.org/jdk pull/10860/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10860

View PR using the GUI difftool:
$ git pr show -t 10860

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10860.diff

bridgekeeper · 2022-10-25T19:38:22Z

👋 Welcome back matsaave! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-10-25T19:41:09Z

@matias9927 The following label will be automatically applied to this pull request:

hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-10-25T21:29:40Z

Webrevs

coleenp · 2022-10-25T21:39:55Z

Can you post an example how it looks for method and field entries ?

matias9927 · 2022-10-25T22:20:02Z

Calling findmethod will result in this output for an invokedynamic call:

(gdb) call findmethod("Concat0", "main", 0x8)
"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

You can see that the flag value is currently decoded twice, once as a quick translation and once in full detail. Printing will look nearly identical for a field but with different flags and no appendix.

dholmes-ora

Hi Matias,

A quick skim through showed a couple of minor issues to fix. I don't really use this output so I can't say whether the expanded format is helpful or too verbose (there are certainly a lot more lines printed now).
Thanks.

src/hotspot/share/oops/cpCache.cpp

matias9927 · 2022-10-26T19:21:18Z

I added a null check for method, a ResourseMark, and I replaced the else if with an else and an assert. Thank you for the corrections!

dholmes-ora

Updates look good. One further nit, but otherwise seems okay. But someone who is actively going to be using this needs to comment on the format/content changes. Thanks.

src/hotspot/share/oops/cpCache.cpp

iklam · 2022-10-27T05:55:50Z

For testing the handling of uninitialized ConstantPoolEntry's, you can do something like this:

set a breakpoint at InstanceKlass::initialize_impl in gdb
when the breakpoint is hit, check if constants()->cache() is NULL
if not NULL, do this: call constants()->cache()->print_on(tty)

This should catch the problem in your earlier commit where you didn't check for the NULL Method pointer.

coleenp

I look at this output.

coleenp · 2022-10-31T18:16:33Z

src/hotspot/share/oops/cpCache.cpp

@@ -690,7 +687,7 @@ void ConstantPoolCacheEntry::print(outputStream* st, int index, const ConstantPo
               flag_state(), is_final(), is_volatile(), field_index());
    st->print_cr(" - tos: %s\n - final: %d\n - volatile: %d\n - field index: %04x",
          type2name(as_BasicType(flag_state())), is_final(), is_volatile(), field_index());
-    //st->print_cr(" - is resolved: %s", pool->tag_at(constant_pool_index()->is_unresolved_klass()) ? "false" : "true");
+    st->print_cr(" - is resolved: %s", pool->tag_at(constant_pool_index()).is_unresolved_klass() ? "false" : "true");


This is confusing and doesn't really help. "is_resolved" means that the bytecode_1 or bytecode_2 is filled in in the indices field. The constant pool index here is a JVM_CONSTANT_Fieldref which points to JVM_CONSTANT_NameAndType and JVM_CONSTANT_{UnresolvedClass,Class,UnresolvedClassInError}. I think this would always print false. I would just leave this line out.

coleenp · 2022-10-31T18:17:29Z

src/hotspot/share/oops/constantPool.cpp

-      st->print(" name_and_type_index=%d", uncached_name_and_type_ref_index_at(index));
+    // print strings not indices
+      //st->print("klass_index=%d", uncached_klass_ref_index_at(index));
+      //st->print(" name_and_type_index=%d", uncached_name_and_type_ref_index_at(index));


Should remove commented out code!

coleenp · 2022-10-31T18:17:40Z

src/hotspot/share/oops/constantPool.cpp

-      st->print("name_index=%d", name_ref_index_at(index));
-      st->print(" signature_index=%d", signature_ref_index_at(index));
+      // st->print("name_index=%d", name_ref_index_at(index));
+      // st->print(" signature_index=%d", signature_ref_index_at(index));


coleenp · 2022-10-31T18:18:59Z

src/hotspot/share/oops/constantPool.hpp

@@ -692,6 +692,30 @@ class ConstantPool : public Metadata {
    resolve_string_constants_impl(h_this, CHECK);
  }

+// Acquire symbols from method and field entries
+// For fields and methods
+char* name_symbol_at(int which) { return symbol_at(name_ref_index_at(uncached_name_and_type_ref_index_at(which)))->as_C_string(); }


Can you align the {'s or use a line break and make these functions "const".

matias9927 · 2022-10-31T18:42:56Z

I look at this output.

I made the mistake of pushing incomplete code which explains the chunks of commented code and other issues. I am currently further extending the information being printed both in the constant pool cache and the constant pool as requested by @iklam.

That being said, I will take your corrections into account as I move forward!

openjdk-notifier · 2022-10-31T20:15:54Z

@matias9927 Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See OpenJDK Developers’ Guide for more information.

coleenp

This looks good. The more verbose field entry information can be added in a further RFE.

openjdk · 2022-10-31T20:28:43Z

@matias9927 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8295893: Improve printing of Constant Pool Cache Entries

Reviewed-by: dholmes, coleenp, iklam

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the master branch:

581133a: 8294696: BufferedInputStream.transferTo should drain buffer when mark set
d8573b2: 8294488: Delete KCMS transforms wrappers
f857f79: 8015739: Background of JInternalFrame is located out of JInternalFrame

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@dholmes-ora, @coleenp, @iklam) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

src/hotspot/share/oops/cpCache.cpp

test/hotspot/gtest/oops/test_cpCache_output.cpp

src/hotspot/share/oops/cpCache.cpp

iklam

LGTM. Just a minor nit.

src/hotspot/share/oops/cpCache.cpp

matias9927 · 2022-11-03T13:17:50Z

/integrate

openjdk · 2022-11-03T13:19:01Z

@matias9927
Your change (at version 05495a9) is now ready to be sponsored by a Committer.

matias9927 · 2022-11-03T22:18:48Z

Thank you for the corrections and feedback @iklam @coleenp @dholmes-ora!
/integrate

openjdk · 2022-11-03T22:20:23Z

@matias9927
Your change (at version 7cfe1f5) is now ready to be sponsored by a Committer.

dholmes-ora

A couple of copyright notice issues need fixing. Thanks.

src/hotspot/share/oops/cpCache.cpp

test/hotspot/gtest/oops/test_cpCache_output.cpp

matias9927 · 2022-11-04T21:41:06Z

/integrate

openjdk · 2022-11-04T21:42:36Z

@matias9927
Your change (at version 5e324d5) is now ready to be sponsored by a Committer.

dholmes-ora

Looks good. Thanks.

coleenp · 2022-11-07T16:12:32Z

/sponsor

openjdk · 2022-11-07T16:15:27Z

Going to push as commit ba303c0.
Since your change was applied there have been 12 commits pushed to the master branch:

f8b2574: 8296137: diags-examples.xml is broken
00d22f6: 8279164: Disable TLS_ECDH_* cipher suites
d634dde: 8295354: Remove G1 incremental non-copy time calculation
8836b92: 8296226: Add constructors (String,Throwable) and (Throwable) to InvalidParameterException
51f8e9b: 8296443: NMT: Remove cmdline_tracking_level handling code
087cedc: 8295261: RISC-V: Support ReductionV instructions for Vector API
556377a: 8296270: Memory leak in ClassLoader::setup_bootstrap_search_path_impl
c2f7638: 8296335: Fix accessibility manual test instruction
91292d5: 8286301: Port JEP 425 to RISC-V
581133a: 8294696: BufferedInputStream.transferTo should drain buffer when mark set
... and 2 more: https://git.openjdk.org/jdk/compare/b847fb687735ae5dff56d12d221556a5218b5bba...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-11-07T16:15:55Z

@coleenp @matias9927 Pushed as commit ba303c0.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

matias9927 added 3 commits October 25, 2022 19:12

8295893: Improve printing of Constant Pool Cache Entries

79a3b3b

Fixed merge conflicts

cda3299

Removed trailing whitespace

3c49e4d

openjdk bot added the hotspot hotspot-dev@openjdk.org label Oct 25, 2022

fixed last trailing whitespace

0a4c53b

matias9927 marked this pull request as ready for review October 25, 2022 21:08

openjdk bot added the rfr Pull request is ready for review label Oct 25, 2022

dholmes-ora suggested changes Oct 25, 2022

View reviewed changes

src/hotspot/share/oops/cpCache.cpp Outdated Show resolved Hide resolved

src/hotspot/share/oops/cpCache.cpp Outdated Show resolved Hide resolved

src/hotspot/share/oops/cpCache.cpp Outdated Show resolved Hide resolved

Added null check and resource mark

f007d46

dholmes-ora reviewed Oct 26, 2022

View reviewed changes

src/hotspot/share/oops/cpCache.cpp Outdated Show resolved Hide resolved

changed NULL to nullptr

452ef59

coleenp reviewed Oct 31, 2022

View reviewed changes

matias9927 force-pushed the invokeDynamicPrinter branch from 83a6cce to 452ef59 Compare October 31, 2022 20:14

coleenp approved these changes Oct 31, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 31, 2022

Added gtest

f2cc110

iklam suggested changes Nov 1, 2022

View reviewed changes

Improved formatting

0315426

iklam approved these changes Nov 1, 2022

View reviewed changes

src/hotspot/share/oops/cpCache.cpp Outdated Show resolved Hide resolved

Fixed code formatting

05495a9

iklam approved these changes Nov 2, 2022

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 3, 2022

Fixed gtest

7cfe1f5

openjdk bot removed the sponsor Pull request is ready to be sponsored label Nov 3, 2022

openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 3, 2022

dholmes-ora suggested changes Nov 4, 2022

View reviewed changes

src/hotspot/share/oops/cpCache.cpp Outdated Show resolved Hide resolved

test/hotspot/gtest/oops/test_cpCache_output.cpp Show resolved Hide resolved

Fixed copyright

d716d2e

openjdk bot removed the sponsor Pull request is ready to be sponsored label Nov 4, 2022

Merge branch 'master' into invokeDynamicPrinter

5e324d5

openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 4, 2022

dholmes-ora approved these changes Nov 4, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Nov 7, 2022

openjdk bot closed this Nov 7, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Nov 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8295893: Improve printing of Constant Pool Cache Entries #10860

8295893: Improve printing of Constant Pool Cache Entries #10860

matias9927 commented Oct 25, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 25, 2022

openjdk bot commented Oct 25, 2022

mlbridge bot commented Oct 25, 2022 •

edited

Loading

coleenp commented Oct 25, 2022

matias9927 commented Oct 25, 2022 •

edited

Loading

dholmes-ora left a comment

matias9927 commented Oct 26, 2022

dholmes-ora left a comment

iklam commented Oct 27, 2022

coleenp left a comment

coleenp Oct 31, 2022

coleenp Oct 31, 2022

coleenp Oct 31, 2022

coleenp Oct 31, 2022

matias9927 commented Oct 31, 2022

openjdk-notifier bot commented Oct 31, 2022

coleenp left a comment

openjdk bot commented Oct 31, 2022 •

edited

Loading

iklam left a comment

matias9927 commented Nov 3, 2022

openjdk bot commented Nov 3, 2022

matias9927 commented Nov 3, 2022

openjdk bot commented Nov 3, 2022

dholmes-ora left a comment

matias9927 commented Nov 4, 2022

openjdk bot commented Nov 4, 2022

dholmes-ora left a comment

coleenp commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

8295893: Improve printing of Constant Pool Cache Entries #10860

8295893: Improve printing of Constant Pool Cache Entries #10860

Conversation

matias9927 commented Oct 25, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Oct 25, 2022

openjdk bot commented Oct 25, 2022

mlbridge bot commented Oct 25, 2022 • edited Loading

Webrevs

coleenp commented Oct 25, 2022

matias9927 commented Oct 25, 2022 • edited Loading

dholmes-ora left a comment

Choose a reason for hiding this comment

matias9927 commented Oct 26, 2022

dholmes-ora left a comment

Choose a reason for hiding this comment

iklam commented Oct 27, 2022

coleenp left a comment

Choose a reason for hiding this comment

coleenp Oct 31, 2022

Choose a reason for hiding this comment

coleenp Oct 31, 2022

Choose a reason for hiding this comment

coleenp Oct 31, 2022

Choose a reason for hiding this comment

coleenp Oct 31, 2022

Choose a reason for hiding this comment

matias9927 commented Oct 31, 2022

openjdk-notifier bot commented Oct 31, 2022

coleenp left a comment

Choose a reason for hiding this comment

openjdk bot commented Oct 31, 2022 • edited Loading

iklam left a comment

Choose a reason for hiding this comment

matias9927 commented Nov 3, 2022

openjdk bot commented Nov 3, 2022

matias9927 commented Nov 3, 2022

openjdk bot commented Nov 3, 2022

dholmes-ora left a comment

Choose a reason for hiding this comment

matias9927 commented Nov 4, 2022

openjdk bot commented Nov 4, 2022

dholmes-ora left a comment

Choose a reason for hiding this comment

coleenp commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

matias9927 commented Oct 25, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 25, 2022 •

edited

Loading

matias9927 commented Oct 25, 2022 •

edited

Loading

openjdk bot commented Oct 31, 2022 •

edited

Loading