[
https://issues.apache.org/jira/browse/GROOVY-12263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105045#comment-18105045
]
ASF GitHub Bot commented on GROOVY-12263:
-----------------------------------------
daniellansun commented on code in PR #2790:
URL: https://github.com/apache/groovy/pull/2790#discussion_r3790871927
##########
src/main/java/groovy/lang/Closure.java:
##########
@@ -1593,7 +1650,26 @@ static CallOverride lookup(Class<?> type) {
}
any = true;
}
- return any ? new CallOverride(byArity, guards, callForm) : NONE;
+ 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]);
+ }
+ }
Review 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 `WrongMethodTypeException`
if the wrong `invokeExact` form were used;
- a single catch-all `(Object, Object[])` slot cannot represent both a 5-arg
and a 6-arg `doCall` (`asSpreader` is arity-specific);
- array-typed / varargs `doCall` still belongs to the MOP (`hasArray` skip),
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.
> Invoke cached Closure doCall targets via MethodHandle
> -----------------------------------------------------
>
> Key: GROOVY-12263
> URL: https://issues.apache.org/jira/browse/GROOVY-12263
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> The {{Closure.call(Object...)}} fast path (GROOVY-11911, GROOVY-12164,
> GROOVY-12165) already caches a per-arity {{doCall}} / {{call}} {{Method}} and
> invokes it with {{Method.invoke}}. That still puts a reflective invoke —
> access check, argument boxing, {{InvocationTargetException}} wrap — on every
> GDK {{each}} / {{collect}} / {{findAll}} / {{inject}} callback from Java.
> h2. Proposal
> At cache-build time, {{MethodHandles.unreflect}} the cached {{Method}} and
> adapt it to {{genericMethodType(arity+1)}}. {{call(Object...)}} then prefers
> {{invokeExact}} on that handle (specialized for arities 0–4).
> {{Method.invoke}} remains only when the method cannot be adapted, so the
> GROOVY-11911 {{call()}} / {{call(Object)}} carve-out still works if unreflect
> fails.
> Exception contracts stay as they were on the reflective path: a body-thrown
> throwable surfaces unwrapped. The handle path must not treat a body-thrown
> {{InvocationTargetException}} or {{IllegalAccessException}} as a reflection
> wrapper.
> Guards, {{CallOverride.NONE}} for {{MethodClosure}} / {{CurriedClosure}}, and
> the metaclass fallback for coercion (GROOVY-12164) are unchanged.
> h2. Why this path
> Java callers such as {{DefaultGroovyMethods}} resolve {{closure.call(item)}}
> to {{Closure.call(Object)}}, which wraps into {{call(Object...)}}. Groovy
> {{invokedynamic}} sites typically bind straight to {{doCall}} after warmup
> and never enter this method — they are out of scope.
> h2. Verification
> Same-host JMH, 4 forks, 99.9% CI, parent {{9bb195dee5}} vs {{1c3820bff7}},
> JDK 25. Host-calibration geomean 0.998x.
> || bench || speedup ||
> | {{eachWithClosure}} | 1.171x |
> | {{collectWithClosure}} | 1.163x |
> | {{findAllWithClosure}} | 1.128x |
> | {{injectWithClosure}} | 1.212x |
> | GDK geomean | 1.168x (~5 ns/callback) |
> | Groovy-indy {{doCall}} sites | 0.990x (flat) |
> | {{MethodClosure}} ({{list.&size}}) | 0.994x (flat) |
> All four GDK 99.9% CIs are disjoint. Full write-up:
> {{docs/closure-call-methodhandle-perf-report.md}}.
> h2. Related
> GROOVY-11911 introduced the reflective cache. GROOVY-12164 / GROOVY-12165
> extended it with typed and multi-arity guards. This change keeps that
> selection and only replaces the invoke.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
