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

[daniellansun]

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

[codecov-commenter]

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]

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]

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]

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><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>
	* The benchmarks in this class cover selected comparisons between:
	* <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>
	* 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]

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.