GROOVY-12263: Invoke cached Closure doCall targets via MethodHandle - #2790
GROOVY-12263: Invoke cached Closure doCall targets via MethodHandle#2790daniellansun wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2790 +/- ##
==================================================
+ Coverage 70.1064% 70.1436% +0.0371%
- Complexity 35774 35808 +34
==================================================
Files 1561 1562 +1
Lines 132373 132484 +111
Branches 24332 24365 +33
==================================================
+ Hits 92802 92929 +127
+ Misses 31170 31154 -16
Partials 8401 8401
🚀 New features to boost your workflow:
|
JMH summary — classic (commit
|
| Group | Speedup | Calibrated | n |
|---|---|---|---|
| bench | 0.989 × | 1.015 × | 99 |
| core | 1.141 × | 1.032 × | 83 |
| grails | 1.008 × | 0.870 × | 80 |
⚠️ 7 benchmarks at least 1.5× slower than the 90-day baseline:
org.apache.groovy.perf.grails.CategoryBench.categoryShadowingExistingMethod— 3.08× slower (calibrated)org.apache.groovy.perf.grails.CategoryBench.nestedCategories— 2.68× slower (calibrated)org.apache.groovy.perf.grails.CategoryBench.categoryInLoop— 2.66× slower (calibrated)org.apache.groovy.perf.grails.CategoryBench.categoryWithOutsideCalls— 2.61× slower (calibrated)org.apache.groovy.perf.grails.CategoryBench.multipleCategoriesSimultaneous— 2.44× slower (calibrated)org.apache.groovy.perf.grails.CategoryBench.nestedCategoryOuterWrapping— 2.37× slower (calibrated)org.apache.groovy.perf.grails.CategoryBench.threeCategoriesSimultaneous— 1.95× slower (calibrated)
⚠️ Runner speed differs ≥15% from the historical baseline hardware for: grails-ad. Raw speedups are not meaningful for those parts — use the calibrated column.
Runner calibration (this run vs baseline hardware): bench 0.98× (26 rulers) · core-ag 1.14× (3 rulers) · core-hz 1.07× (3 rulers) · grails-ad 1.43× (3 rulers) · grails-ez 0.98× (3 rulers)
Baseline: dev/bench/jmh/<part>/classic/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data
JMH summary — indy (commit
|
| Group | Speedup | Calibrated | n |
|---|---|---|---|
| bench | 0.973 × | 0.998 × | 99 |
| core | 2.762 × | 2.670 × | 83 |
| grails | 5.276 × | 3.942 × | 80 |
No benchmark is ≥1.5× slower than its 90-day baseline.
⚠️ Runner speed differs ≥15% from the historical baseline hardware for: grails-ad, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.
Runner calibration (this run vs baseline hardware): bench 0.97× (26 rulers) · core-ag 1.12× (3 rulers) · core-hz 0.93× (3 rulers) · grails-ad 1.26× (3 rulers) · grails-ez 1.41× (3 rulers)
Baseline: dev/bench/jmh/<part>/indy/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data
| case 4: | ||
| return handle.invokeExact((Object) self, arguments[0], arguments[1], arguments[2], arguments[3]); | ||
| default: | ||
| return handle.asSpreader(Object[].class, arguments.length).invokeExact((Object) self, arguments); |
There was a problem hiding this comment.
I think you have no advantage of invokeExact in this case, unless you cache the handle resulting from asSpreader. If not cached I would use invokeWithArguments here instead. I doubt it is slower for this case, but it sure is better readable.
There was a problem hiding this comment.
Agreed — thank you.
invokeExact only helps when the call site sees a stable MethodType. A per-call asSpreader allocates a new handle, so that invokeExact was just a more expensive invoke, and less readable than invokeWithArguments.
We took the other half of the same advice: the spreader is now built once in CallOverride.unreflect and stored as (Object, Object[])Object (MethodType.genericMethodType(1, true)). invokeHandle's default is then:
return handle.invokeExact((Object) self, arguments);That is a cached spreader, so invokeExact is the type-correct call. We did not use handle.invokeWithArguments(self, arguments): that is a real varargs method and would pack as length 2.
The specialised 0..4 invokeExact switch is unchanged. That remains the GDK each / collect / inject path.
| if (byArity[arity] != null) { | ||
| handles[arity] = unreflect(byArity[arity]); | ||
| } | ||
| } |
There was a problem hiding this comment.
the cases over arity limit could be created here as well, but with taking Object[]
There was a problem hiding this comment.
Agreed, and that is now in place.
ARITY_LIMIT is only the specialised-invokeExact cutoff (0..4). doCall methods with arity >= 5 are no longer skipped. Each unambiguous, non-array target is stored in a small side table (SpreadSlot[]), keyed by exact parameter count, as a lookup-time asSpreader of type (Object, Object[])Object.
We kept that table off the handles[0..4] array on purpose:
- mixing the two handle shapes in one array would
WrongMethodTypeExceptionif the wronginvokeExactform were used; - a single catch-all
(Object, Object[])slot cannot represent both a 5-arg and a 6-argdoCall(asSpreaderis arity-specific); - array-typed / varargs
doCallstill belongs to the MOP (hasArrayskip), as before.
Selection, GROOVY-12164 guards, ambiguity, MethodClosure / CurriedClosure → NONE, and the mopUnperturbed gate are the same rules as for 0..4. High-arity is always a doCall body, so it does not use the GROOVY-11911 re-entry latch.
The 0..4 branch in call(Object...) (arity < ARITY_LIMIT) is unchanged, so the measured GDK path is not on this table.
Build asSpreader handles once at CallOverride lookup for arity >= 5, and invokeExact the cached (Object, Object[])Object shape. Leave the specialised 0-4 invokeExact switch on the GDK each/collect path unchanged.
|
✅ All tests passed ✅🏷️ Commit: 9b75750 Learn more about TestLens at testlens.app. |



https://issues.apache.org/jira/browse/GROOVY-12263
Performance Verification Report
Subject.
1c3820bff71419b5c40e73142bbf3e82210ea317— Invoke cached Closure doCall targets via MethodHandleBaseline.
9bb195dee52e82518bb5f4e5cda9ddbf08c16d39— immediate parent of the subject. The only production delta issrc/main/java/groovy/lang/Closure.java. The accompanying unit-test file is not on the JMH hot path.Verdict. The change delivers a statistically significant, host-stable, and path-specific improvement on the Java/GDK
Closure.callentry that the commit claims to accelerate.On the four GDK iteration benches that actually execute
DefaultGroovyMethods→Closure.call(Object)→call(Object...), wall-clock time falls by 13–21% (geometric mean 1.168×, about 4–6 ns per callback). Groovyinvokedynamiccall sites that already bind straight todoCallare unchanged (geomean 0.990×). AMethodClosurenegative control is unchanged (0.994×). Host-calibration rulers sit at 0.998×, so the GDK movement is not a host-speed artifact.1. What was compared
git diff --stat 9bb195dee5 1c3820bff7:src/main/java/groovy/lang/Closure.javadoCall/calltargets are invoked viaMethodHandle.invokeExactinstead ofMethod.invoke.src/test/groovy/groovy/lang/ClosureCallHandleTest.groovyNo other module, Gradle flag, or benchmark source differs between the two worktrees.
Isolated bytecode check (same class extracted from each JMH fat JAR):
Closure.classSHA-256invokeExactcountMethod.invokecount7426d0a0…5461d6092475a3f1…3e76a198The six
invokeExactsites are the specialized 0–4-arity cases plus the defensive spreader. The remainingMethod.invokeis the documented fallback whenunreflectfails. The two JARs therefore implement the two dispatch strategies under test and nothing else.2. Measurement protocol
The experiment is a paired, same-host, same-JVM, sequential comparison of two isolated builds. It is deliberately not a comparison against the 90-day gh-pages history: that history is dominated by runner-hardware noise (see
subprojects/performance/README.adoc).2.1 Isolation
/tmp/groovy-base-9bb195d,/tmp/groovy-mh-1c3820b)../gradlew :performance:jmhJar --offline.performance-6.0.0-SNAPSHOT-jmh.jar).2.2 JMH configuration
These flags override the
@Fork(2)annotations on the benchmark classes.org.apache.groovy.perf.ClosureBench(19 methods) +org.apache.groovy.perf.HostCalibrationBench(3 rulers)AverageTime, as declared by the benches (ms/opforClosureBench,us/opfor rulers)-Xms2g -Xmx2g -XX:+AlwaysPreTouch2.3 Host
hera25.0.2-amzn)cpufreqn/a)2.4 How “faster” is defined
For
AverageTime, speedup = baseline / target. Values greater than 1 mean the target is faster. A result is labeled faster or slower only when the two 99.9% CIs do not overlap; otherwise inconclusive.A Welch two-sample t on the 20 raw iteration samples is reported as a secondary check (
t, Welch–Satterthwaitedf). Two-sided p-values are not tabulated: SciPy is not installed on this host, and every GDK t exceeds 11 ondf > 22, which isp ≪ 0.001under any reasonable tail model.3. Path analysis: which benches must move
The production change lives only in
Closure.call(Object...). A bench can improve only if its steady-state work actually enters that method.Generated closures declare
doCall(...)and do not overridecall(Object). Two distinct caller shapes then arise:Java / GDK entry.
DefaultGroovyMethods.each/collect/findAll/injectcompile as Javaclosure.call(item)(orcall(acc, val)). That resolves toClosure.call(Object)/call(Object, Object), which wrap intocall(Object...). This is the path named in the commit comment (theeach/collecthot path). Primary treatment.Groovy
invokedynamicentry.c(i)andc.call(i)inClosureBenchcompile to the same indy site,invoke:(Lgroovy/lang/Closure;I). After warmup the site binds directly todoCall(Object)and never entersClosure.call(Object...). Must not move. Confirmed by disassembly ofClosureBench.closureCallMethodandclosureReusein the target JAR, and by the generated classClosureBench$_closureCallMethod_closure14exposing onlydoCall/doCall().Adapters that re-enter a generated closure.
CurriedClosureandMethodClosureare explicitlyCallOverride.NONE.ComposedClosure.doCall(Object[])is array-typed and likewise uncached. Their outer call stays on the metaclass. The inner generated bodies, however, are invoked viacall(...)after uncurry or composition, so they can pick up theMethodHandlepath. These are secondary / indirect, not clean negatives.True negative control.
list.&sizeis aMethodClosure:CallOverride.lookupreturnsNONE, and there is no generateddoCallbody to re-enter. Must not move.Hardware rulers.
HostCalibrationBench.{cpuIntegerOps, memoryPointerChase, allocationChurn}are pure Java and Groovy-independent. Their geometric mean is the calibration factor. A factor near 1 means the two 29-minute windows ran at equivalent host speed.4. Results
4.1 Hardware calibration (must be ~1.0×)
cpuIntegerOpsmemoryPointerChaseallocationChurnThe second window is 0.2% slower on the geometric mean of the rulers — well inside JMH noise, and in the opposite direction of the GDK result. A 17% GDK movement cannot be attributed to the machine speeding up.
4.2 Primary treatment — GDK Java callbacks (must improve if the claim is true)
Each of these methods performs 1 000 000 closure invocations per JMH op (
ITERATIONS/10outer loops × a 10-element list, or 2-arginjectover the same list). Scores are therefore also nanoseconds per callback, including iterator and DGM overhead.eachWithClosurecollectWithClosurefindAllWithClosureinjectWithClosurePer-fork means (ms/op) — every fork of every GDK bench moves in the same direction; this is not a single lucky fork:
eachcollectfindAllinjectRelative CI half-width stays in the 1.3–4.4% band on both sides; the target is if anything tighter.
findAllsaves a little less (~3.9 ns) thaneach/collect/inject(~5–6 ns), which is consistent withBooleanClosureWrapperadding a fixed cost that theMethodHandlechange does not touch.Reading the 5 ns. A Groovy-indy
doCallof{ it * 2 }is ~2.2 ns in the same process (closureCallMethod). The GDK benches spend ~34 ns per callback, of which iterator + DGM +call(Object)array wrap +doCallbody account for the rest. RemovingMethod.invoke’s reflective wrapper from that mix and replacing it withinvokeExactis expected to save a handful of nanoseconds, not tens. The measured −5 ns/call matches that model. It is not a 17% reduction insidedoCallitself; it is a 17% reduction in the GDK callback round-trip, which is exactly the surface the commit optimizes.4.3 Groovy indy sites (must not improve)
closureCallMethodclosureReuseclosureWithCaptureclosureModifyCaptureclosureMultiParamssimpleClosureCreationclosureAsParameterclosureDelegationnestedClosuresnestedClosuresandclosureDelegationhave wide CIs (inner allocation and property-dispatch work dominate) and still overlap. The near-2.2 nsclosureCallMethod/closureReusepair is identical to 0.1%. This group is the specificity check: if the whole JVM had simply gotten faster, these would have moved with the GDK benches. They did not.4.4 Adapters and the true negative control
methodReferenceMethodClosure→NONEcurriedClosureNONE, inner re-enterscallrightCurriedClosureclosureCompositiondoCall(Object[])uncached; inners viacallmethodReferenceis the clean negative and does not move (t = −0.47). Curry, rcurry, and compose do move, in the same 16–25% band as the GDK group. That is expected once the inner generated closures are taken into account:CurriedClosureis excluded from the cache so thatMetaClassImplcan uncurry and re-enter, and that re-entry is acall(...)on a generated closure. Treating them as proof of a general, unspecified speedup would be wrong; treating them as a contradiction would also be wrong. They are a consistent secondary effect.4.5 Context benches (not a test of this commit)
closureTrampolineclosureSpreadTrampoline is dominated by
TrampolineClosuremachinery. Spread (sum3(*args)) is dominated by argument packing. CIs overlap.5. Why this is the MethodHandle change, not a confound
cpuIntegerOpstick is smaller than, and opposite in direction to, a 17% GDK claim.git diffis two files; both JARs built--offlinefrom worktrees pinned at the two SHAs;Closure.classhashes andinvokeExactcounts match the intended implementations.doCallsites andMethodClosuredid not move. The improvement is confined to callers that enterClosure.call(Object...).ClosureCallHandleTestis not referenced fromClosureBench.@PackedClosures/groovy.target.closure.pack).ClosureBenchis compiled without it; the generated classes extendClosureand implementGeneratedClosurewithdoCallonly.6. What this report does not claim
c(i)was already ~2.2 ns/call and stays there.doCall. The ~5 ns is the reflective-invoke overhead disappearing from the Javacallwrapper arounddoCall.-prof gcwas not attached, to keep the timing run clean), or packed-closure dispatch (PackedClosureoverridescalland never uses this cache).7. Conclusion
On a same-host, 4-fork, 99.9%-CI JMH comparison of the isolated parent and the MethodHandle commit:
each/collect/findAll/injectimprove by 1.13–1.21× (geomean 1.168×, −4 to −6 ns per callback). All four 99.9% CIs are disjoint; all four fork sets move in the same direction.doCall(geomean 0.990×) andMethodClosure(0.994×) do not move.call(...); they are not counterexamples.The performance claim of
1c3820bis therefore confirmed for the Java/GDKClosure.callhot path, at a magnitude that matches the cost ofMethod.invokebeing removed, and with no detectable regression on the paths the change is designed to leave alone.Appendix A — Reproducing this run
Raw JMH JSON from this run:
/tmp/grok-1000/jmh-base.json,/tmp/grok-1000/jmh-target.json.Appendix B — Artifact hashes
9bb195dee52e82518bb5f4e5cda9ddbf08c16d391c3820bff71419b5c40e73142bbf3e82210ea317e321d268c7e099519e3f9e8b1b38d9bba5e11bf8151ee928f4a12156d4e400164cd0b63d1ca9a7f9cb383bae2b594adfc3cb079790260d3415aa2308d5f02b17