daniellansun commented on code in PR #2790:
URL: https://github.com/apache/groovy/pull/2790#discussion_r3790866273
##########
src/main/java/groovy/lang/Closure.java:
##########
@@ -594,6 +586,59 @@ public V call(final Object... arguments) {
}
}
+ /**
+ * Invokes a cached {@code doCall}/{@code call} target. Prefers the adapted
+ * {@link MethodHandle} so {@code Method.invoke} is not on the GDK
+ * {@code each}/{@code collect} hot path. Exceptions thrown by the body —
+ * including a body that itself throws {@link InvocationTargetException} or
+ * {@link IllegalAccessException} — are rethrown as-is on the handle path;
+ * the {@link Method#invoke} fallback unwraps only the wrapper
+ * {@link InvocationTargetException} that reflection introduces.
+ */
+ @SuppressWarnings("unchecked")
+ private static <V> V invokeCached(final MethodHandle handle, final Method
target, final Closure<?> self, final Object[] arguments) {
+ if (handle != null) {
+ try {
+ return (V) invokeHandle(handle, self, arguments);
+ } catch (Throwable t) {
+ UncheckedThrow.rethrow(t);
+ return null;
+ }
+ }
+ try {
+ return (V) target.invoke(self, arguments);
+ } catch (InvocationTargetException ite) {
+ UncheckedThrow.rethrow(ite.getCause());
+ return null; // unreachable statement
+ } catch (IllegalAccessException iae) {
+ throw new GroovyRuntimeException(iae);
+ }
+ }
+
+ /**
+ * {@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.
+ */
+ private static Object invokeHandle(final MethodHandle handle, final
Closure<?> self, final Object[] arguments) throws Throwable {
+ switch (arguments.length) {
+ case 0:
+ return handle.invokeExact((Object) self);
+ case 1:
+ return handle.invokeExact((Object) self, arguments[0]);
+ case 2:
+ return handle.invokeExact((Object) self, arguments[0],
arguments[1]);
+ case 3:
+ return handle.invokeExact((Object) self, arguments[0],
arguments[1], arguments[2]);
+ 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);
Review 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:
```java
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
