You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/08/18 00:49:07 UTC
(groovy) 03/03: GROOVY-12263: Cache high-arity doCall targets as MethodHandle spreaders
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 763cbbf2bd6d467f9c4aeeec4fe0ef70e06e17a7
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 16 12:37:50 2026 +0900
GROOVY-12263: Cache high-arity doCall targets as MethodHandle spreaders
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.
---
src/main/java/groovy/lang/Closure.java | 157 ++++++++++++++++++---
.../groovy/lang/ClosureCallHandleTest.groovy | 97 +++++++++++--
2 files changed, 226 insertions(+), 28 deletions(-)
diff --git a/src/main/java/groovy/lang/Closure.java b/src/main/java/groovy/lang/Closure.java
index fdc5b294f6..8f0f4d611f 100644
--- a/src/main/java/groovy/lang/Closure.java
+++ b/src/main/java/groovy/lang/Closure.java
@@ -48,10 +48,13 @@ import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayDeque;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.Deque;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiConsumer;
@@ -560,6 +563,13 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
if (m != null && CallOverride.guardsPass(override.guards[arity], arguments)) {
target = m;
}
+ } else {
+ // High-arity doCall: cached asSpreader of type (Object, Object[])Object.
+ // The 0..ARITY_LIMIT-1 invokeExact switch is unchanged.
+ CallOverride.SpreadSlot slot = override.spreadFor(arity);
+ if (slot != null && CallOverride.guardsPass(slot.guards, arguments)) {
+ return invokeCached(slot.handle, slot.method, this, arguments);
+ }
}
if (target != null) {
MethodHandle handle = override.handles[arity];
@@ -616,11 +626,12 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
}
/**
- * {@code invokeExact} against a handle adapted to
- * {@link MethodType#genericMethodType(int) genericMethodType(arity+1)}
- * (fixed-arity {@code Object} receiver and arguments, {@code Object} return).
- * Cases {@code 0..ARITY_LIMIT-1} match that type exactly; the spreader
- * is the type-correct fallback if the limit grows without a matching case.
+ * {@code invokeExact} against a cached handle. Cases {@code 0..ARITY_LIMIT-1}
+ * match {@link MethodType#genericMethodType(int) genericMethodType(arity+1)}.
+ * The default is a handle already adapted to
+ * {@link MethodType#genericMethodType(int, boolean) genericMethodType(1, true)}
+ * ({@code (Object, Object[])Object}) at lookup — {@code asSpreader} is not
+ * applied per call.
*/
private static Object invokeHandle(final MethodHandle handle, final Closure<?> self, final Object[] arguments) throws Throwable {
switch (arguments.length) {
@@ -635,7 +646,7 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
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);
+ return handle.invokeExact((Object) self, arguments);
}
}
@@ -1490,14 +1501,22 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
private static final class CallOverride {
private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
/**
- * Cached arities: {@code 0..ARITY_LIMIT-1}. Sized to cover every callback shape the GDK
- * drives (iteration and inject/withIndex variants peak at three parameters, plus slack).
+ * Specialised {@code invokeExact} cutoff: cases {@code 0..ARITY_LIMIT-1} use a
+ * fixed-arity handle. Sized to cover every callback shape the GDK drives
+ * (iteration and inject/withIndex variants peak at three parameters, plus slack).
+ * Higher arities are cached separately as {@link #SPREADER_TYPE} handles.
*/
static final int ARITY_LIMIT = 5;
+ /**
+ * Cached high-arity shape: {@code (Object, Object[])Object}. Built once at
+ * lookup with {@code asSpreader}; {@code invokeHandle} then {@code invokeExact}s it.
+ */
+ static final MethodType SPREADER_TYPE = MethodType.genericMethodType(1, true);
+ static final SpreadSlot[] NO_SPREAD = new SpreadSlot[0];
/**
* Marker instance indicating that no dispatch targets exist.
*/
- static final CallOverride NONE = new CallOverride(new Method[ARITY_LIMIT], new Class[ARITY_LIMIT][], new boolean[ARITY_LIMIT], new MethodHandle[ARITY_LIMIT]);
+ static final CallOverride NONE = new CallOverride(new Method[ARITY_LIMIT], new Class[ARITY_LIMIT][], new boolean[ARITY_LIMIT], new MethodHandle[ARITY_LIMIT], NO_SPREAD);
/**
* Cached {@code doCall} targets indexed by parameter count, or null where absent,
* ambiguous, or inaccessible — {@code call} selects a {@code doCall}, so the cache is
@@ -1530,14 +1549,49 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
* for {@code invokeExact}, or null when the method cannot be adapted.
* The call path prefers the handle so {@code Method.invoke} is not on
* the GDK {@code each}/{@code collect} hot path.
+ * Slots {@code 0..ARITY_LIMIT-1} are {@code genericMethodType(arity+1)};
+ * higher arities live in {@link #spread}.
*/
final MethodHandle[] handles;
+ /**
+ * Exact-arity {@code doCall} targets with {@code arity >= ARITY_LIMIT},
+ * each adapted to {@link #SPREADER_TYPE}. Empty when the class has none.
+ */
+ final SpreadSlot[] spread;
- private CallOverride(Method[] byArity, Class<?>[][] guards, boolean[] callForm, MethodHandle[] handles) {
+ private CallOverride(Method[] byArity, Class<?>[][] guards, boolean[] callForm, MethodHandle[] handles, SpreadSlot[] spread) {
this.byArity = byArity;
this.guards = guards;
this.callForm = callForm;
this.handles = handles;
+ this.spread = spread;
+ }
+
+ /**
+ * One high-arity {@code doCall}: exact parameter count, optional 12164
+ * guards, and a lookup-time spreader handle.
+ */
+ static final class SpreadSlot {
+ final int arity;
+ final Method method;
+ final Class<?>[] guards;
+ final MethodHandle handle;
+
+ SpreadSlot(final int arity, final Method method, final Class<?>[] guards, final MethodHandle handle) {
+ this.arity = arity;
+ this.method = method;
+ this.guards = guards;
+ this.handle = handle;
+ }
+ }
+
+ SpreadSlot spreadFor(final int arity) {
+ for (SpreadSlot slot : spread) {
+ if (slot.arity == arity) {
+ return slot;
+ }
+ }
+ return null;
}
static boolean guardsPass(Class<?>[] guards, Object[] args) {
@@ -1551,8 +1605,10 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
/**
* Finds the subclass's {@code doCall} declarations, one target per arity — {@code call}
- * selects a {@code doCall}, so the cache is keyed on the doCall declarations themselves
- * and any visibility the MOP would dispatch is accepted (the hierarchy is walked with
+ * selects a {@code doCall}, so the cache is keyed on the doCall declarations themselves.
+ * Arities {@code 0..ARITY_LIMIT-1} use specialised {@code invokeExact}; higher arities
+ * are stored as {@link #SPREADER_TYPE} handles. Any visibility the MOP would dispatch
+ * is accepted (the hierarchy is walked with
* {@code getDeclaredMethods}; an overridden signature keeps its most-derived form).
* Bridge methods are skipped (their erased twin is the real declaration), as are static
* declarations and methods with array-typed parameters (that shape belongs to vararg
@@ -1583,6 +1639,9 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
Method[] exact = new Method[ARITY_LIMIT];
Method[] typed = new Method[ARITY_LIMIT];
boolean[] typedAmbiguous = new boolean[ARITY_LIMIT];
+ Map<Integer, Method> highExact = null;
+ Map<Integer, Method> highTyped = null;
+ Set<Integer> highAmbiguous = null;
Set<String> seen = new HashSet<>();
for (Class<?> c = type; c != null && c != Closure.class; c = c.getSuperclass()) {
for (Method m : c.getDeclaredMethods()) {
@@ -1592,13 +1651,29 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
Class<?>[] params = m.getParameterTypes();
if (!seen.add(java.util.Arrays.toString(params))) continue; // overridden below: most-derived wins
int arity = m.getParameterCount();
- if (arity >= ARITY_LIMIT) continue;
boolean allObject = true, hasArray = false;
for (Class<?> p : params) {
if (p != Object.class) allObject = false;
if (p.isArray()) hasArray = true;
}
if (hasArray) continue;
+ if (arity >= ARITY_LIMIT) {
+ if (highExact == null) {
+ highExact = new HashMap<>();
+ highTyped = new HashMap<>();
+ highAmbiguous = new HashSet<>();
+ }
+ if (allObject) {
+ highExact.putIfAbsent(arity, m);
+ } else if (highExact.containsKey(arity)) {
+ // all-Object at this arity already won, as for 0..ARITY_LIMIT-1
+ } else if (highTyped.containsKey(arity)) {
+ highAmbiguous.add(arity);
+ } else {
+ highTyped.put(arity, m);
+ }
+ continue;
+ }
if (allObject) {
if (exact[arity] == null) exact[arity] = m;
} else if (typed[arity] != null) {
@@ -1650,22 +1725,66 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
}
any = true;
}
- if (!any) {
- return NONE;
- }
MethodHandle[] handles = new MethodHandle[ARITY_LIMIT];
for (int arity = 0; arity < ARITY_LIMIT; arity += 1) {
if (byArity[arity] != null) {
handles[arity] = unreflect(byArity[arity]);
}
}
- return new CallOverride(byArity, guards, callForm, handles);
+ SpreadSlot[] spread = buildSpread(highExact, highTyped, highAmbiguous);
+ if (spread.length > 0) {
+ any = true;
+ }
+ if (!any) {
+ return NONE;
+ }
+ return new CallOverride(byArity, guards, callForm, handles, spread);
+ }
+
+ private static SpreadSlot[] buildSpread(final Map<Integer, Method> highExact, final Map<Integer, Method> highTyped, final Set<Integer> highAmbiguous) {
+ if (highExact == null) {
+ return NO_SPREAD;
+ }
+ Set<Integer> arities = new HashSet<>(highExact.keySet());
+ arities.addAll(highTyped.keySet());
+ List<SpreadSlot> built = new ArrayList<>(arities.size());
+ for (Integer boxed : arities) {
+ int arity = boxed;
+ Method exact = highExact.get(boxed);
+ Method m = (exact != null) ? exact
+ : (highAmbiguous.contains(boxed) ? null : highTyped.get(boxed));
+ if (m == null) {
+ continue;
+ }
+ try {
+ m.setAccessible(true);
+ } catch (RuntimeException ignored) {
+ continue;
+ }
+ Class<?>[] g = null;
+ if (m != exact) {
+ Class<?>[] params = m.getParameterTypes();
+ g = new Class[arity];
+ for (int i = 0; i < arity; i += 1) {
+ g[i] = (params[i] == Object.class) ? null : wrapperOf(params[i]);
+ }
+ }
+ built.add(new SpreadSlot(arity, m, g, unreflect(m)));
+ }
+ return built.isEmpty() ? NO_SPREAD : built.toArray(NO_SPREAD);
}
private static MethodHandle unreflect(final Method method) {
try {
- return LOOKUP.unreflect(method)
- .asType(MethodType.genericMethodType(method.getParameterCount() + 1));
+ int arity = method.getParameterCount();
+ MethodHandle handle = LOOKUP.unreflect(method)
+ .asType(MethodType.genericMethodType(arity + 1));
+ if (arity >= ARITY_LIMIT) {
+ // box first, then spread: asSpreader on a still-primitive handle
+ // would demand a primitive array
+ handle = handle.asSpreader(Object[].class, arity).asType(SPREADER_TYPE);
+ }
+ return handle;
} catch (Exception ignored) {
// inaccessible or not adaptable: call() falls back to Method.invoke
return null;
diff --git a/src/test/groovy/groovy/lang/ClosureCallHandleTest.groovy b/src/test/groovy/groovy/lang/ClosureCallHandleTest.groovy
index 6c4deb092d..0d07d2f711 100644
--- a/src/test/groovy/groovy/lang/ClosureCallHandleTest.groovy
+++ b/src/test/groovy/groovy/lang/ClosureCallHandleTest.groovy
@@ -138,11 +138,59 @@ final class ClosureCallHandleTest {
}
@Test
- void 'arity at the cache limit is not cached'() {
+ void 'arity at the specialised limit is cached as a spreader'() {
def five = { a, b, c, d, e -> a + b + c + d + e }
assert javaVarargsCall(five, 1, 2, 3, 4, 5) == 15
- assert cachedTarget(five, 5) == null
- assert handleFor(five, 5) == null
+ assert cachedTarget(five, 5) != null
+ MethodHandle handle = handleFor(five, 5)
+ assert handle != null
+ assert handle.type() == MethodType.genericMethodType(1, true)
+ shouldFail(MissingMethodException) {
+ javaVarargsCall(five, 1, 2, 3, 4)
+ }
+ shouldFail(MissingMethodException) {
+ javaVarargsCall(five, 1, 2, 3, 4, 5, 6)
+ }
+
+ def six = { a, b, c, d, e, f -> a + b + c + d + e + f }
+ assert javaVarargsCall(six, 1, 2, 3, 4, 5, 6) == 21
+ assert handleFor(six, 6).type() == MethodType.genericMethodType(1, true)
+ assert handleFor(six, 5) == null
+
+ def fiveInts = new Closure(this) {
+ int doCall(int a, int b, int c, int d, int e) { a + b + c + d + e }
+ }
+ assert javaVarargsCall(fiveInts, 1, 2, 3, 4, 5) == 15
+ assert handleFor(fiveInts, 5) != null
+
+ def boom = { a, b, c, d, e -> throw new IllegalStateException('five') }
+ def e = shouldFail(IllegalStateException) {
+ javaVarargsCall(boom, 1, 2, 3, 4, 5)
+ }
+ assert e.message == 'five'
+ }
+
+ @Test
+ void 'typed high-arity doCall still falls through when a guard fails'() {
+ def c = new Closure(this) {
+ def doCall(String a, String b, String c, String d, String e) {
+ a + b + c + d + e
+ }
+ }
+ assert javaVarargsCall(c, 'a', 'b', 'c', 'd', 'e') == 'abcde'
+ assert handleFor(c, 5) != null
+ assert javaVarargsCall(c, 'a', 'b', 'c', 'd', "${'e'}") == 'abcde'
+ }
+
+ @Test
+ void 'high-arity call override without doCall is not cached'() {
+ def c = new Closure(this) {
+ Object call(a, b, c, d, e) { 'wrapped' }
+ }
+ shouldFail(MissingMethodException) {
+ javaVarargsCall(c, 1, 2, 3, 4, 5)
+ }
+ assert handleFor(c, 5) == null
}
@Test
@@ -272,11 +320,11 @@ final class ClosureCallHandleTest {
def c = new Closure(this) {
def doCall(a, b, d, e, f) { [a, b, d, e, f] }
}
- Method method = findDoCall(c.getClass(), 5)
- method.accessible = true
- MethodHandle handle = MethodHandles.lookup().unreflect(method)
- .asType(MethodType.genericMethodType(6))
+ MethodHandle handle = handleFor(c, 5)
+ assert handle != null
+ assert handle.type() == MethodType.genericMethodType(1, true)
assert invokeHandleDirect(handle, c, [1, 2, 3, 4, 5] as Object[]) == [1, 2, 3, 4, 5]
+ assert javaVarargsCall(c, 1, 2, 3, 4, 5) == [1, 2, 3, 4, 5]
}
@Test
@@ -535,12 +583,43 @@ final class ClosureCallHandleTest {
return (arity < slots.length) ? slots[arity] : null
}
+ private static int arityLimit() {
+ def f = Class.forName('groovy.lang.Closure$CallOverride').getDeclaredField('ARITY_LIMIT')
+ f.accessible = true
+ return f.getInt(null)
+ }
+
private static MethodHandle handleFor(Closure closure, int arity) {
- return (MethodHandle) callOverrideField(closure, 'handles', arity)
+ if (arity < arityLimit()) {
+ return (MethodHandle) callOverrideField(closure, 'handles', arity)
+ }
+ return (MethodHandle) spreadSlotField(closure, arity, 'handle')
}
private static Method cachedTarget(Closure closure, int arity) {
- return (Method) callOverrideField(closure, 'byArity', arity)
+ if (arity < arityLimit()) {
+ return (Method) callOverrideField(closure, 'byArity', arity)
+ }
+ return (Method) spreadSlotField(closure, arity, 'method')
+ }
+
+ private static Object spreadSlotField(Closure closure, int arity, String field) {
+ Class<?> callOverride = Class.forName('groovy.lang.Closure$CallOverride')
+ Object override = callOverrideOf(closure)
+ def spread = callOverride.getDeclaredField('spread')
+ spread.accessible = true
+ Object[] slots = (Object[]) spread.get(override)
+ Class<?> slotType = Class.forName('groovy.lang.Closure$CallOverride$SpreadSlot')
+ def arityField = slotType.getDeclaredField('arity')
+ arityField.accessible = true
+ def valueField = slotType.getDeclaredField(field)
+ valueField.accessible = true
+ for (Object slot : slots) {
+ if (arityField.getInt(slot) == arity) {
+ return valueField.get(slot)
+ }
+ }
+ return null
}
private static Method findDoCall(Class<?> type, int arity) {