[
https://issues.apache.org/jira/browse/GROOVY-12164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18096100#comment-18096100
]
ASF GitHub Bot commented on GROOVY-12164:
-----------------------------------------
Copilot commented on code in PR #2710:
URL: https://github.com/apache/groovy/pull/2710#discussion_r3576680527
##########
src/main/java/groovy/lang/Closure.java:
##########
@@ -1422,5 +1446,51 @@ private static Method findOverride(Class<?> type,
Class<?>... params) {
return null;
}
}
+
+ /**
+ * Finds the single unambiguous one-argument {@code call} override
with a declared
+ * (non-Object) parameter type (GROOVY-12164). Array-typed parameters
are skipped —
+ * that shape belongs to vararg collection, which is metaclass work —
as are bridge
+ * methods (their erased twin is the real override) and overloaded
typed overrides
+ * (ambiguous: the metaclass performs the selection).
+ *
+ * @param type the closure subclass
+ * @return the override, or null when absent, ambiguous, or
inaccessible
+ */
+ private static Method findTypedOneArgOverride(Class<?> type) {
+ Method candidate = null;
+ for (Method m : type.getMethods()) {
+ if (m.getParameterCount() != 1 || !"call".equals(m.getName())
+ || m.isBridge() || m.getDeclaringClass() ==
Closure.class) {
+ continue;
+ }
Review Comment:
findTypedOneArgOverride can currently treat a public *static* call(T) method
as the cached instance override (because getMethods() includes static methods).
If a Closure subclass happens to declare a static call(T), the fast path would
invoke it even though it is not an instance override, changing behavior vs the
metaclass doCall fallback. Filter out static methods (and optionally synthetic
ones) when selecting the typed candidate.
> Closure.call fast path misses closures with typed parameters (~6x slower via
> full metaclass dispatch)
> -----------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12164
> URL: https://issues.apache.org/jira/browse/GROOVY-12164
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> Description:
> The Closure.call(Object...) fast path added in GROOVY-11911 caches a direct
> doCall/call Method per closure subclass (CallOverride, keyed via
> {{type.getMethod("call", Object.class)}}). A generated closure class only
> declares a {{call(Object)}} override when its doCall takes Object — i.e. when
> the closure parameter is untyped ({{it}} or {{ { x -> } }}). A closure with a
> *typed* parameter generates {{doCall(Integer)}}/{{call(Integer)}}, which the
> Object-signature lookup cannot see, so CallOverride resolves to NONE and every
> invocation falls back to full {{getMetaClass().invokeMethod(this, "doCall",
> args)}} dispatch.
> Since callers dispatch through the static type Closure (e.g. every DGM
> iteration method calls {{closure.call(item)}} from Java), the typed
> {{call(Integer)}} overload on the generated class is never selected either.
> Net effect: annotating a closure parameter with its type — normally good
> practice — costs roughly 3-6x in per-element dispatch overhead.
> Measurements (steady-state, median of 9 trials, isolated JVM per variant,
> 12-element List, @CompileStatic enclosing class, JDK 23, master):
> || closure || param || capture || ops/ms ||
> | {{ { x -> s += (int) x } }} (each) | untyped | Reference | 13,408 |
> | {{ { it * it } }} (collect) | untyped | none | 13,173 |
> | {{ { Integer x -> x * x } }} (collect) | typed | none | 4,148 |
> | {{ { Integer x -> s += x } }} (collect) | typed | Reference | 3,606 |
> | {{ { Integer x -> a[0] += x } }} (each) | typed | int[] | 2,471 |
> | {{ { Integer x -> s += x } }} (each) | typed | Reference | 2,229 |
> The typed/untyped split is the dominant factor; capture kind and
> each-vs-collect are second-order.
> Possible fixes (either restores the fast path for typed params):
> 1. Extend CallOverride.lookup to also accept a single-arg typed override
> (resolve the closure's declared one-arg call/doCall whatever its parameter
> type), with an argument compatibility/coercion guard before the cached
> reflective invoke so metaclass coercion semantics (GString->String,
> number conversions, null handling) are preserved, falling back to
> invokeMethod on mismatch.
> 2. Have ClosureWriter always emit a coercing {{call(Object)}} bridge alongside
> the typed doCall (castToType to the declared parameter type, then direct
> doCall), so the existing Object-signature lookup finds every generated
> closure class.
> Option 2 keeps the runtime lookup untouched and localises the semantics in
> generated code, but adds a method per closure class; option 1 is
> runtime-only and also covers pre-existing compiled classes.
> Discovered while benchmarking GROOVY-12151 (closure packing): the packed
> adapter dispatches typed parameters via checkcast/unbox in a generated
> dispatch table and is unaffected, which initially made generated closure
> classes look artificially slow in the typed-parameter comparison.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
