Hi Jochen,
AI read below. It could be wrong. I am still pondering but won't get
time again until my tomorrow. So this is something to read in the
meantime.
Cheers, Paul.
--------
no disagreement with the conceptual core — doCall is the semantic source of
truth, call is its polymorphic entry, and PackedClosure needs explicit MOP
standing rather than the accidental one it has. But before improvements are
designed, two factual findings are worth having on the table, because they
change what "aligning with the contract" means in practice.
1) The doCall-only contract has never held on the dynamic path.
I tested a Closure subclass declaring only `String call(String)` (no doCall)
across releases:
dynamic cl("foo") GDK collect(cl)
Groovy 3.0.21-25 works MissingMethodException
Groovy 4.0.32 works MissingMethodException
Groovy 5.0.6 works MissingMethodException
Groovy 6.0.0-alpha-1 works MissingMethodException
master (12164/65) works works
So ClosureMetaClass has dispatched declared call overloads for at least four
major versions; only the Java entry (Closure.call -> invokeMethod("doCall"))
enforced doCall-only, meaning the same cl("foo") succeeded from Groovy code
and failed from each(). What GROOVY-12164/12165 changed is that the Java path
now matches the dynamic path — the same direction GROOVY-11911 already took
for call(Object)-without-doCall subclasses. Your doCall expectations
themselves hold exactly on master: with doCall(int), cl(1) works and
cl("foo") is a MissingMethodException on both paths (the typed guards decline
to the metaclass, which coerces or fails as before).
That leaves a genuine decision rather than a bug report: either custom call
overloads are accepted as the (now consistent) de-facto contract, or
doCall-only is enforced on BOTH paths — which is a behavioural break with
3.x-6.x dynamic dispatch. If the latter, the mechanical cost is small: the
CallOverride cache re-keys on doCall overloads instead of call overloads
(keeping 11911's call(Object) compatibility case), and none of the measured
wins move, because generated closure classes' call(T) bridges mirror their
doCall(T) one-to-one — the cache just changes lookup key. Happy to do that if
we agree doCall-only is the contract.
2) On PackedClosure, I agree with each observation, with one correction and
one caveat:
- "only one doCall analog possible": already handled by declining — a closure
with default parameter values (the multiple-doCall case) is not packable
and keeps its class. It is extensible later (one table id per generated
arity) if ever wanted.
- not a GeneratedClosure: conceded — and notably the naive fix is unsound.
ClosureMetaClass resolves the "this" type for GeneratedClosure by walking
getEnclosingClass(), which assumes closure classes are inner classes of the
owner; PackedClosure is a top-level adapter, so the marker would break that.
Your sentence "strictly seen PackedClosure requires its own logic in
MetaClass" is exactly right, and I'd rather add that explicit handling than
overload the marker.
- one shared MetaClass for all packed closures: conceded, and it is the one
semantic difference that cannot be engineered away — per-literal metaclass
is definitionally what packing removes. Class-level metaClass changes on
PackedClosure would affect every packed closure. The delegate/
resolveStrategy axis is already fenced by the runtime guard; this axis
needs documenting as a caveat of the opt-in feature (and is one more reason
packing must stay off for code that plays MOP games).
- "the call path seems to be the only way to invoke the real method": true,
but I'd frame what that path now IS: a statically compiled, metaclass-free,
inlinable route (adapter -> per-arity dispatch table -> direct invocation of
the typed hoisted body). For statically compiled callers, invokevirtual
call() on a packed closure reaches a typed body with no MOP involvement at
all — which is the closest thing we currently have to your point (6),
since the typed doCall of a closure class is hidden behind Closure<T> for
static compilation, but a packed closure's entry is structurally fixed.
Recent measurements: packed closures now run 1.8-4.7x FASTER than generated
closure classes on capturing shapes with byte-identical behaviour, largely
because of this property.
- one point in the architecture's favour under your criterion (2): dynamic
call sites invoking closures are megamorphic across per-literal closure
classes, but monomorphic on the single PackedClosure class — the per-target
selection moves into a switch the JIT handles well. Indy caching gets
easier, not harder.
So: agreement on the concepts, two concrete workstreams if we also agree on
the remedies — (a) decide the custom-call question and, if doCall-only wins,
re-key the CallOverride cache on doCall for both paths; (b) give PackedClosure
explicit ClosureMetaClass (and Selector) handling plus documentation of the
shared-metaclass caveat. I'd take both.
On Tue, Jul 14, 2026 at 8:09 PM Jochen Theodorou <[email protected]> wrote:
>
> Hi all,
>
>
> when looking at https://github.com/apache/groovy/pull/2710 I was
> thinking that something is going wrong here on a conceptual level.
>
> we have quite a few cases to consider here, but in general (in no order
> really):
> (1) we do not want to call invokeMethod, getProperty or setProperty
> directly if we can avoid it
> (2) for dynamic Groovy this implies caching using invokedynamic and
> getting the information from the meta class now
> (3) GeneratedClosure allows a simplified path since we know there is no
> custom MOP logic for these with default MetaClass
> (4) call is supposed to be used to call doCall only
> (5) providing custom call methods is actually not intended
> (6) static compilation needs to call doCall directly
> (7) static compilation needs to can't go through getProperty or setProperty
> (8) a Closure may define multiple doCall methods.
>
> What I mean with 4+5.
> If you define subclass Closure and provide a call(String) method and you
> do a call like cl("foo") (cl is the instance of the custom Closure),
> then this call is supposed to fail. There is no doCall method at all.
> If you subclass Closure and provide a doCall(int) method then cl(1) is
> supposed to work, but cl("foo") still fails since there is no doCall
> method fitting that argument.
>
> Which leads to (6)
> The big problem for static compilation is that it this doCall method
> might be hidden if we just provide Closure<T> as type. And that means
> static compilation may have to fall back to dynamic invocation for that
> - or go through call, which does a kind of dynamic invocation. So number
> (6) (and probably 7) can currently actually not be realized.
>
> Now we added PackedClosure, which is very similar to GeneratedClosure,
> except that there is no doCall method and the method is instead in the
> host class, plus there is currently only one doCall analog possible plus
> nothing in the PackedClosure may escape the Closure. Also, PackedClosure
> replaces GeneratedClosure in some cases, without being a
> GeneratedClosure, so the MOP behaviour is different for the two - which
> could lead to problems. And again here the call-path seems to be the
> only way to invoke the real method. Strictly seen PackedClosure requires
> its own logic in MetaClass. And unlike GeneratedClosure, all
> PackedClosure have only 1 MetaClass, which they share
>
> But it is not only static compilation and the usage of PackedClosure
> that have trouble with this architecture it is also invocation through
> Java, especially our GDK.
>
> Before making any suggestions about improvements - does somebody
> disagree with this?
>
> bye Jochen
