GROOVY-11935: Set invokedynamic call site target immediately to enable earlier JIT inlining by daniellansun · Pull Request #2473 · apache/groovy

daniellansun · 2026-04-18T13:03:51Z

https://issues.apache.org/jira/browse/GROOVY-11935

codecov-commenter · 2026-04-18T13:28:04Z

Codecov Report

❌ Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.3214%. Comparing base (4a57272) to head (28da458).
⚠️ Report is 79 commits behind head on master.

Files with missing lines	Patch %	Lines
...org/codehaus/groovy/vmplugin/v8/IndyInterface.java	95.6522%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2473        +/-   ##
==================================================
+ Coverage     67.1472%   67.3214%   +0.1742%     
- Complexity      30944      33513      +2569     
==================================================
  Files            1438       1451        +13     
  Lines          120148     128714      +8566     
  Branches        21311      24107      +2796     
==================================================
+ Hits            80676      86652      +5976     
- Misses          32724      34904      +2180     
- Partials         6748       7158       +410

Files with missing lines	Coverage Δ
...codehaus/groovy/vmplugin/v8/CacheableCallSite.java	`78.4615% <ø> (+1.5385%)`	⬆️
...dehaus/groovy/vmplugin/v8/MethodHandleWrapper.java	`100.0000% <100.0000%> (ø)`
...org/codehaus/groovy/vmplugin/v8/IndyInterface.java	`91.1111% <95.6522%> (+2.4748%)`	⬆️

... and 76 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Implements GROOVY-11935 by adjusting the invokedynamic dispatch path to relink static method call sites earlier (to enable earlier JIT inlining), and adds a dedicated JMH benchmark to measure the impact.

Changes:

Add early MutableCallSite.setTarget for static method calls when the receiver is a Class.
Extend MethodHandleWrapper to carry the resolved MetaMethod so static-ness can be detected at the call site.
Introduce new JMH benchmarks (StaticMethodCallIndyBench + Groovy helper) to compare Java vs Groovy dynamic vs @CompileStatic.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
subprojects/performance/src/jmh/groovy/org/apache/groovy/bench/StaticMethodCallIndyBench.java	New JMH benchmark targeting early relinking for static indy calls.
subprojects/performance/src/jmh/groovy/org/apache/groovy/bench/StaticMethodCallIndy.groovy	Helper methods (dynamic, instance, and `@CompileStatic`) used by the new benchmark.
src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java	Adds `MetaMethod` storage/access so callers can inspect modifiers (e.g., `static`).
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java	Adds GROOVY-11935 early relinking logic and passes `selector.method` into `MethodHandleWrapper`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T23:27:03Z

+        // GROOVY-11935: Set invokedynamic call site target immediately for static method calls to enable earlier JIT inlining
+        if (receiver instanceof Class) {
+            var method = mhw.getMethod();
+            if (method != null && Modifier.isStatic(method.getModifiers())) {
+                callSite.setTarget(mhw.getTargetMethodHandle());
+            }
+        }


The new GROOVY-11935 behavior changes call-site relinking (static calls can now update the MutableCallSite target immediately). There are existing tests for IndyInterface’s deprecated entry points, but none appear to assert the new static-call relinking behavior or that it only happens when caching/guards are enabled. Please add a regression test that exercises a static call through IndyInterface.fromCache/fromCacheHandle and verifies the call site target is (or is not) updated as intended.

Copilot · 2026-04-18T23:27:03Z

+        if (receiver instanceof Class) {
+            var method = mhw.getMethod();
+            if (method != null && Modifier.isStatic(method.getModifiers())) {
+                callSite.setTarget(mhw.getTargetMethodHandle());


The early callSite.setTarget() path for static calls does not check mhw.isCanSetTarget(). If Selector.cache is false (e.g., UNCACHED_CALL / guardless paths), this can install an unguarded target handle and bypass switchpoint/arg guards, changing semantics and potentially breaking invalidation. Gate this optimization on mhw.isCanSetTarget() (and ideally also avoid resetting the target if it’s already the desired one).

Suggested change

if (receiver instanceof Class) {

var method = mhw.getMethod();

if (method != null && Modifier.isStatic(method.getModifiers())) {

callSite.setTarget(mhw.getTargetMethodHandle());

if (mhw.isCanSetTarget() && receiver instanceof Class) {

var method = mhw.getMethod();

if (method != null && Modifier.isStatic(method.getModifiers())) {

var targetMethodHandle = mhw.getTargetMethodHandle();

if (callSite.getTarget() != targetMethodHandle) {

callSite.setTarget(targetMethodHandle);

}

Copilot · 2026-04-18T23:27:03Z

+ * This benchmark compares:
+ * <ul>
+ *   <li><b>Java</b> — direct {@code invokestatic}, the theoretical optimum</li>
+ *   <li><b>Groovy dynamic</b> — {@code invokedynamic} with early setTarget</li>
+ *   <li><b>Groovy {@code @CompileStatic}</b> — direct {@code invokestatic}</li>
+ *   <li><b>Groovy instance</b> — {@code invokedynamic} without early setTarget (control group)</li>
+ * </ul>


This Javadoc claims the benchmark compares 4 variants (including Groovy instance control group and a Groovy @CompileStatic baseline), but the actual benchmark methods don’t cover all variants for each scenario (e.g., no instance Fibonacci benchmark and no @CompileStatic chained-call benchmark). Please align the list with what’s actually measured, or add the missing benchmark methods so the documentation matches the results.

Suggested change

* This benchmark compares:

* <ul>

* <li>Java — direct {@code invokestatic}, the theoretical optimum</li>

* <li>Groovy dynamic — {@code invokedynamic} with early setTarget</li>

* <li>Groovy {@code @CompileStatic} — direct {@code invokestatic}</li>

* <li>Groovy instance — {@code invokedynamic} without early setTarget (control group)</li>

* </ul>

* The benchmarks in this class cover selected comparisons between:

* <ul>

* <li>Java — direct {@code invokestatic}, the theoretical optimum</li>

* <li>Groovy dynamic — {@code invokedynamic} with early setTarget</li>

* <li>Groovy {@code @CompileStatic} — direct {@code invokestatic}</li>

* <li>Groovy instance — {@code invokedynamic} without early setTarget (control group)</li>

* </ul>

* Not every scenario includes every variant; each benchmark method measures

* only the implementations defined for that specific case.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

blackdrag · 2026-04-20T00:48:04Z

     */
    private static MethodHandle fromCacheHandle(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
        FallbackSupplier fallbackSupplier = new FallbackSupplier(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
+        final Object receiver = arguments[0];


So what will happen in this case?

def foo(receiver){receiver.bar()} class A{ static bar(){1} } class B{ static bar(){2} } foo(A) foo(B) foo(new A()) foo(new B())

for the first two calls the class is the same, since receiver.getClass will return Class for both. But that is imho a bug, that existed already before callSite.getAndPut(receiverClassName,.... Also I think the later two cases are not considered here... why not? If we really only want to optimize A.bar() style calls, then we should check the receiver type through the callsite type, not the runtime type.

…e earlier JIT inlining

daniellansun · 2026-04-22T13:57:51Z

If no one is opposed to the PR, I will merge it at this weekend.

daniellansun requested a review from paulk-asert April 18, 2026 13:04

paulk-asert requested a review from Copilot April 18, 2026 23:21

Copilot started reviewing on behalf of paulk-asert April 18, 2026 23:21 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

asf-gitbox-commits force-pushed the GROOVY-11935 branch 3 times, most recently from eef7dfb to 3c7055f Compare April 19, 2026 04:04

daniellansun requested a review from Copilot April 19, 2026 04:11

Copilot started reviewing on behalf of daniellansun April 19, 2026 04:11 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Comment thread src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java

asf-gitbox-commits force-pushed the GROOVY-11935 branch 4 times, most recently from df2f9f7 to bd517da Compare April 19, 2026 16:18

blackdrag reviewed Apr 20, 2026

View reviewed changes

asf-gitbox-commits force-pushed the GROOVY-11935 branch from bd517da to 42bd244 Compare April 20, 2026 15:48

daniellansun changed the title ~~GROOVY-11935: Set invokedynamic call site target immediately for static method calls to enable earlier JIT inlining~~ GROOVY-11935: Set invokedynamic call site target immediately to enable earlier JIT inlining Apr 20, 2026

asf-gitbox-commits force-pushed the GROOVY-11935 branch 3 times, most recently from e8bb30f to a13afa5 Compare April 20, 2026 22:35

daniellansun requested a review from blackdrag April 21, 2026 00:04

GROOVY-11935: Set invokedynamic call site target immediately to enabl…

28da458

…e earlier JIT inlining

asf-gitbox-commits force-pushed the GROOVY-11935 branch from a13afa5 to 28da458 Compare April 21, 2026 18:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GROOVY-11935: Set invokedynamic call site target immediately to enable earlier JIT inlining#2473

GROOVY-11935: Set invokedynamic call site target immediately to enable earlier JIT inlining#2473
daniellansun wants to merge 1 commit intomasterfrom
GROOVY-11935

daniellansun commented Apr 18, 2026

Uh oh!

codecov-commenter commented Apr 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

blackdrag Apr 20, 2026

Uh oh!

daniellansun commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

daniellansun commented Apr 18, 2026

Uh oh!

codecov-commenter commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

blackdrag Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

daniellansun commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Apr 18, 2026 •

edited

Loading