8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool #25420

teshull · 2025-05-23T17:37:14Z

This PR adds support for directly retrieving both all invokedynamic and all condy BootstrapMethodInvocations from a ConstantPool via the new method List<BootstrapMethodInvocation> lookupBootstrapMethodInvocations(boolean invokeDynamic).

In addition, two methods are added to the BootstrapMethodInvocations:

void resolve()
JavaConstant lookup()

The combination of these two features allows one to directly interact with all BSM information of a given ConstantPool without having to iterate through all of the Classfile's methods to find all invokedynamic bytecodes and/or iterate through all Constant Pool entries.

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-8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool (Enhancement - P4)

Reviewers

Doug Simon (@dougxc - Reviewer)
Yudi Zheng (@mur47x111 - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25420

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…onstant Pool.

bridgekeeper · 2025-05-23T17:38:03Z

Hi @teshull, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user teshull" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

openjdk · 2025-05-23T17:38:35Z

@teshull 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:

8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool

Reviewed-by: dnsimon, yzheng

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 12 new commits pushed to the master branch:

a653ff4: 8358536: jdk/jfr/api/consumer/TestRecordingFileWrite.java times out
248341d: 8344942: Template-Based Testing Framework
09ec4de: 8358066: Non-ascii package names gives compilation error "import requires canonical name"
... and 9 more: https://git.openjdk.org/jdk/compare/b5cfd76c047392788b6a5c25ebadc463b2c8ce90...master

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 (@dougxc, @mur47x111) 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).

openjdk · 2025-05-23T17:39:04Z

@teshull The following labels will be automatically applied to this pull request:

graal
hotspot-compiler

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

teshull · 2025-05-23T17:52:02Z

/covered

bridgekeeper · 2025-05-23T17:52:49Z

Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

dougxc · 2025-05-24T08:49:54Z

Please add some tests for the new methods to test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestDynamicConstant.java.

teshull · 2025-05-26T13:39:06Z

@dougxc I integrated testing for the new methods into TestDynamicConstant.java now

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java

dougxc · 2025-05-26T14:14:33Z

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java

+     * constant pool contains no invokedynamic BootstrapMethodInvocations, then null
+     * is returned.
+     */
+    BootstrapMethodInvocation[] lookupAllIndyBootstrapMethodInvocations();


Why not make this return all BootstrapMethodInvocations? The caller can then filter out the indy ones with isInvokeDynamic. Also, please return a List<BootstrapMethodInvocation> instead of an array - we should never return arrays from JVMCI (see #23159 as an example of addressing existing API). Lastly, return List.of() instead of null.

Changed to return a list.

Why not make this return all BootstrapMethodInvocations

Within HotSpot it is very easy to pick off all indy BootstrapMethodInvocations via the ConstantPoolCache

Each invokedynamic bytecode location has a unique BootstrapMethodInvocation instance, but they may share the same constant pool entry, so it's not trivial to find all BootstrapMethodInvocations. One would have to iterate both all method bytecodes and constant pool slots, and do some additional filtering.

How about List<BootstrapMethodInvocation> lookupBootstrapMethodInvocations(boolean indy)? That is, it either gets the indy or the condy BSM invocations. I can imagine SVM wanting the latter at some point right?

BTW, I noticed that the javadoc for ConstantPool.lookupBootstrapMethodInvocation is somewhat incorrect. Please check and apply these corrections in this PR:

diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java index 2273b256f03..3519af4bcbb 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java @@ -199,12 +199,12 @@ interface BootstrapMethodInvocation { * in the constant pool. * * @param index if {@code opcode} is -1, {@code index} is a constant pool index. Otherwise {@code opcode} - * must be {@code Bytecodes.INVOKEDYNAMIC}, and {@code index} must be the operand of that - * opcode in the bytecode stream (i.e., a {@code rawIndex}). - * @param opcode must be {@code Bytecodes.INVOKEDYNAMIC}, or -1 if + * must be {@code Bytecodes.INVOKEDYNAMIC} or {@code CONSTANT_Dynamic_info}, and {@code index} + * must be the operand of that opcode in the bytecode stream (i.e., a {@code rawIndex}). + * @param opcode must be {@code Bytecodes.INVOKEDYNAMIC}, {@code CONSTANT_Dynamic_info}, or -1 if * {@code index} was not decoded from a bytecode stream * @return the bootstrap method invocation details or {@code null} if the entry specified by {@code index} - * is not a {@code CONSTANT_Dynamic_info} or @{code CONSTANT_InvokeDynamic_info} + * is not a {@code CONSTANT_Dynamic_info} or {@code CONSTANT_InvokeDynamic_info} * @jvms 4.7.23 The {@code BootstrapMethods} Attribute */ default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int opcode) {

I prototyped the option List<BootstrapMethodInvocation> lookupBootstrapMethodInvocations(boolean indy) here: master...teshull:jdk:jvmci_bootstrap_alternative

As part of this I also prototyped generic BSM resolution / lookup logic

From the SVM perspective, retrieving condys via this new support isn't a big win. It's easy enough already to walk the ConstantPool. However, for symmetry purposes, it is reasonable to have this method (along with the resolve / lookup). What's your preference: this new version or the original?

I like the symmetry of the new version. Also, I think you can simplify things by replacing use of flatMap here with filter.

I updated the javadoc misplaced @ in {@code}. However, the opcode doc changes look wrong to me; the opcode must be -1 or INVOKEDYNAMIC (

jdk/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java

Line 592 in 04e0fe0

int cpi = opcode == -1 ? index : indyIndexConstantPoolIndex(index, opcode);

)

yeah, looks like you're right. I was basing my assumption on case "Dynamic" in:

@Override public BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int opcode) { int cpi = opcode == -1 ? index : indyIndexConstantPoolIndex(index, opcode); final JvmConstant tag = getTagAt(cpi); switch (tag.name) { case "InvokeDynamic": case "Dynamic":

I guess it's possible for an INVOKEDYNAMIC to resolve it's cpi to a CONSTANT_Dynamic entry.

I think INVOKEDYNAMIC should always point to a CONSTANT_InvokeDynamic entry

dougxc · 2025-05-28T17:38:15Z

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java

    }

+    /**
+     * Returns the BootstrapMethodInvocation instances for all invokedynamic


Point out that the returned list is unmodifiable (like the API for Stream.toList() does).

teshull · 2025-05-29T06:19:39Z

@dougxc I cleaned up the PR to now have the symmetric lookup option and updated the tests

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java

Co-authored-by: Douglas Simon <[email protected]>

dougxc · 2025-05-29T20:55:25Z

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java

        } else {
-            return IntStream.range(1, length()).filter(cpi -> {
+            return IntStream.range(1, length())
+                            .filter(this::isDynamicEntry)


Looks like you forgot to add the definition of isDynamicEntry that I suggested:

private boolean isDynamicEntry(int cpi) { JvmConstant tagAt = getTagAt(cpi); return tagAt != null && tagAt.name.equals("Dynamic"); }

Yes, I applied the suggested change via github, and am just validating it works now (which of course it doesn't). I'll fix it

dougxc · 2025-05-30T08:54:02Z

I also updated the title of https://bugs.openjdk.org/browse/JDK-8357660 to Not Be All Capitalized so you'll need to fix the title of this PR.
Also, please update both titles and descriptions further to reflect the final changes (i.e. lookupBootstrapMethodInvocations instead of lookupIndyBootstrapMethodInvocations).

dougxc · 2025-05-30T08:57:14Z

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java

        private final String name;
        private final JavaConstant type;
        private final List<JavaConstant> staticArguments;
+        private final int index;


index -> cpiOrIndyIndex

mlbridge · 2025-05-30T15:06:28Z

Webrevs

dougxc

Looks good to me.

Please enable GitHub Actions on your JDK fork.

mur47x111

LGTM

teshull · 2025-06-04T13:45:16Z

/integrate

openjdk · 2025-06-04T13:46:44Z

@teshull
Your change (at version c7f5c1a) is now ready to be sponsored by a Committer.

dougxc · 2025-06-04T13:49:04Z

/sponsor

openjdk · 2025-06-04T13:50:40Z

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

a653ff4: 8358536: jdk/jfr/api/consumer/TestRecordingFileWrite.java times out
248341d: 8344942: Template-Based Testing Framework
09ec4de: 8358066: Non-ascii package names gives compilation error "import requires canonical name"
... and 9 more: https://git.openjdk.org/jdk/compare/b5cfd76c047392788b6a5c25ebadc463b2c8ce90...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-06-04T13:50:52Z

@dougxc @teshull Pushed as commit 0352477.

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

Add support for retrieving all Indy BootstrapMethodInvocations from C…

072467d

…onstant Pool.

bridgekeeper bot added the oca Needs verification of OCA signatory status label May 23, 2025

openjdk bot added graal [email protected] hotspot-compiler [email protected] labels May 23, 2025

bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label May 23, 2025

style fixes and add testing to TestDynamicConstants.

5d868f7

dougxc reviewed May 26, 2025

View reviewed changes

address reviewer feedback

99d785d

dougxc reviewed May 28, 2025

View reviewed changes

change to allow both indys and condys to be looked up all at once

fa98e7a

dougxc reviewed May 29, 2025

View reviewed changes

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java Outdated Show resolved Hide resolved

dougxc reviewed May 29, 2025

View reviewed changes

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java Outdated Show resolved Hide resolved

teshull and others added 2 commits May 29, 2025 21:20

commit review suggestion

2557614

Co-authored-by: Douglas Simon <[email protected]>

commit review suggestion

b8123ba

Co-authored-by: Douglas Simon <[email protected]>

dougxc reviewed May 29, 2025

View reviewed changes

complete changes

519be17

dougxc reviewed May 30, 2025

View reviewed changes

bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels May 30, 2025

openjdk bot added the rfr Pull request is ready for review label May 30, 2025

reviewer feedback and update javadoc formatting

60c39b5

teshull changed the title ~~8357660: [JVMCI] Add Support for Retrieving All Indy BootstrapMethodInvocations directly from the ConstantPool~~ 8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool Jun 2, 2025

dougxc approved these changes Jun 2, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 2, 2025

teshull added 2 commits June 2, 2025 21:08

commit to trigger testing

2639bb1

commit to trigger testing

4d508fc

graalvmbot mentioned this pull request Jun 3, 2025

[JDK-8358545] Adapt "JDK-8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool" oracle/graal#11319

Merged

teshull added 2 commits June 3, 2025 19:31

Merge remote-tracking branch 'origin/master' into JDK-8357660

e0707fb

Merge remote-tracking branch 'origin/master' into JDK-8357660

c7f5c1a

mur47x111 approved these changes Jun 4, 2025

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Jun 4, 2025

openjdk bot added the integrated Pull request has been integrated label Jun 4, 2025

openjdk bot closed this Jun 4, 2025

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 Jun 4, 2025

graalvmbot mentioned this pull request Jun 10, 2025

[GR-65610] Update labsjdk to 25+26-jvmci-b01 oracle/graal#11374

Merged

8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool #25420

8357660: [JVMCI] Add support for retrieving all BootstrapMethodInvocations directly from ConstantPool #25420

Uh oh!

Conversation

teshull commented May 23, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented May 23, 2025

Uh oh!

openjdk bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented May 23, 2025

Uh oh!

teshull commented May 23, 2025

Uh oh!

bridgekeeper bot commented May 23, 2025

Uh oh!

dougxc commented May 24, 2025

Uh oh!

teshull commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull commented May 29, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougxc commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlbridge bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

dougxc left a comment

Choose a reason for hiding this comment

Uh oh!

mur47x111 left a comment

Choose a reason for hiding this comment

Uh oh!

teshull commented Jun 4, 2025

Uh oh!

openjdk bot commented Jun 4, 2025

Uh oh!

dougxc commented Jun 4, 2025

teshull commented May 23, 2025 •

edited by openjdk bot

Loading

openjdk bot commented May 23, 2025 •

edited

Loading

teshull May 27, 2025 •

edited

Loading

teshull May 27, 2025 •

edited

Loading

teshull May 29, 2025 •

edited

Loading

dougxc commented May 30, 2025 •

edited

Loading

mlbridge bot commented May 30, 2025 •

edited

Loading