On 7/5/26 00:16, Paul King wrote:
Thanks Jochen,
Some AI comments below in two parts.
What did you use for that (Agent, LLM model)? I did not read the
comments yet, just wondering if we use the same stuff here. After
reading I am pretty sure that is a no. Still I am curious about the
tooling. I am mostly using cline with whatever is currently a free
model, plus also chatgpt chats and other things.
Is there any free for OSS work models out there? They really help
supporting summarizing and analyzing as well as solution finding in
general, even if the agent does not change the code in the end.
[...]
Meanwhile post-JEP-416 reflection is fast cold precisely because the
whole JVM shares a handful of per-arity LambdaForms. Groovy's design
is the opposite: maximal shape diversity, paid up front, per site.
while the I omitted points before are not wrong, the just show we build
a very complex handle. This resulting paragraph is the actual problem.
> ## The plan — four workstreams
### W0 (do first): a noise-free cold-path metric
CI-runner timing noise is drowning your signal, but there's a metric
that is *deterministic*: **the number of LambdaForm/hidden classes
spun during a canonical workload**. Count
`LambdaForm$MH`/`LambdaForm$DMH`/hidden class definitions (JFR
`jdk.ClassLoad`, or a `-verbose:class` parse, or loaded-class-count
deltas) while running a fixed script corpus. Every improvement in
W1–W3 shows up as a monotone drop in that counter, immune to runner
variance. Add alongside it:
- an **instance-dispatch cold bench** (clone of
`StaticMethodCallIndyColdBench`, which is currently the *only*
single-shot bench, and it only covers statics),
- an **end-to-end "fresh JVM runs a realistic script" bench** — that's
the number users actually feel, and none of the 30-odd JMH benches
measures it.
This is a week of work and it de-risks everything below.
To understand the basic problem I was doing benchmarks on a much lower
level. How much does creating a Callsite in invokedynamic cost? How much
a single uncached MethodHandle, how much a single reflective call. How
much doing this twice? Those are really microbenchmarks. But if they say
one is 15 times slower than the other and one has repeating cost because
of different usages and the other not... well then a pattern forms. And
that is even before we do the Groovy based logic
### W1: contained wins on the current architecture
1. **Per-`CachedMethod` MethodHandle cache** (Jochen's "receiver
view", minimal form). Cache the unreflected handle on `CachedMethod`
via `SoftReference`, mirroring the existing
`pogo/pojo/staticCallSiteConstructor` pattern. The subtlety:
`unreflect` goes through the caller lookup, so the cached handle is
caller-independent only for public methods on public, accessible
classes and **never for `@CallerSensitive` methods** (unreflect binds
the lookup class into those). Gate the cache on `isPublic(method) &&
isPublic(declaringClass)` plus a caller-sensitivity check
(annotation-name probe with a "don't cache" fallback). The
`GeneratedMetaMethod.getTargetMethodHandle()` mechanism from
GROOVY-12069 is the precedent — this extends the same idea from
generated DGMs to all methods.
Of course I have thought about this too, I am a bit afraid of this
meaning we pay with longer startup/metaclass init times. The more
methods with different signatures the higher the cost most likely. But I
have not tested it. Adding a MethodHandle to CachedMethod would,
unfortunately kind of change the public API. As for the lookup object...
it depends on what you use. In a bootstrap scenario we have the lookup
of the caller class. If we are going to make a call to a private method
on the same class, then this is very possible. There is also the
question of JPMS. Just making every method available that Groovy has
read rights to is not going to make Groovy a proper member in that
infrastructure.
2. **Adaptive/early promotion for instance sites.** GROOVY-11935
promotes statics on first hit; instance sites wait 1000 calls. A
monomorphic instance site whose PIC key repeats could promote after a
handful of hits — the guard chain already protects correctness and
`groovy.indy.fallback.cutoff` already protects against deopt storms.
Cheap experiment: sweep `groovy.indy.optimize.threshold` with the cold
bench, then implement "promote early once the same wrapper hits k
times".
I think the problem is already there on the first call.
3. **Adapter-layer audit in `Selector`.** Make `explicitCastArguments`
conditional on the types actually differing; check whether
`catchException` can be skipped for more cases than the current
number-method/DGM exceptions; order guards cheapest-first. Each layer
removed is one fewer LambdaForm shape per site.
catchException... did I not add skipping that for some CachedMethod
cases? I have it in my draft PR. It caused a lot of trouble though.
### W2: collapse MethodType shape diversity
Change `InvokeDynamicWriter` to erase **reference types to `Object`**
in indy descriptors while keeping primitives (boxing avoidance) and
the `Wrapper` cast-marker (semantics). Selection is driven by
*runtime* receiver class anyway, so static reference-type
specialization buys almost nothing at selection time — but it's what
makes every site's boot handle, guard chain, and
`explicitCastArguments` a novel shape. Post-erasure, all
`(Object,Object)Object`-shaped sites share LambdaForms JVM-wide. Old
compiled classes keep working because they call the same bootstrap
with their old descriptors. Groovy 6.0 is exactly the release where a
bytecode-format change like this is possible. Measure with the W0
counter — I'd expect this to be the single biggest LambdaForm-count
reduction.
My theory is that if you have have (Foo,int)Object on the callsite (like
when we call a method on Foo, that takes int) and the method we call
returns int, then we have a mismatch of what the callsite requires and
what the target requires. This is then solved by asType or
explicitCastArguments, but it changes the lambda form, causing us to
have to pay the cost for it. Ass I said, a theory. If that is no changed
to (Object,Object)Object then yes, we save on the incoming MethodType
variety, but we still will produce many different lambda forms in the end.
### W3: reflective cold tier (the architecture change Jochen is circling)
Restructure the cold tier so it **doesn't build MethodHandle chains at
all**: the boot target becomes a plain-Java dispatcher (`asCollector`
to `Object[]` is the only MH machinery), the PIC caches the selected
`MetaMethod` plus cheap plain-Java validity checks (metaclass
identity, SwitchPoint validity, category flag — all trivially
checkable outside MH guards), and invocation during the cold phase
goes through `CachedMethod.invoke` → `Method.invoke`, i.e. the
reflection path Jochen measured as 10–15× faster cold. Only at
promotion does the full guarded handle chain get built and `setTarget`
— so the hot path is *byte-for-byte unchanged*.
This is "a Handle calling reflection" from his mail, made concrete.
Caveats to design in: caller-sensitive and JPMS-inaccessible targets
must bypass the reflective tier and use the existing unreflect path
(the `Java9.transformMetaMethod` machinery already identifies the
inaccessible cases); exception unwrapping semantics must match
`catchException(GroovyRuntimeException, …)`. Build it behind
`groovy.indy.cold.reflection` so both tiers ship in 6.0 betas and the
daily indy/classic dashboards can grow a third series to compare. If
it proves out, it also *shrinks* the pressure on W1.2 — a cheap cold
tier makes the 1000-call threshold much less painful.
specifically I am considering doing a template class for a hidden nested
class to do the call to reflection, which would handle the logic as
well. But this comes at a cost of course and I have to check if it is
worth it.
### W4: stop fighting the one-time cost — cache it across runs
Jochen is right that he's fighting unwinnable JVM internals *within a
single JVM run*. But Leyden ships exactly the counter-tool: the **AOT
cache (JEP 483/514/515, JDK 24+)** archives loaded classes, resolved
constant pool state, and profiles from a training run — including spun
LambdaForms. For Grails apps, Gradle builds, and test suites (the
workloads where Groovy cold cost hurts most), a training-run +
AOT-cache recipe may recover more wall-clock time than any runtime
cleverness. Concretely: verify Groovy's generated classes and indy
bootstraps are AOT-cache-friendly, measure `groovy run` / Grails
startup with and without, document it, and consider a `groovy`
launcher flag. Low risk, doesn't touch dispatch code, and it compounds
with W1–W3.
That I have not looked into yet. Especially since I do not know how and
when handles project to what is cached. If it is classes it may not help
with the cold state, since classes are not used right away for
handles... at least as far as I know.
### Track 2 (parallel, correctness not speed): hidden classes + JPMS
Replace `CallSiteGenerator`/`ClassLoaderForClassArtifacts.defineClass`
and `DgmConverter` loading with `defineHiddenClass(..., NESTMATE)` —
not DgmConverter.
[...]
## Suggested sequencing
W0 first (it's cheap and everything else needs it) → W1.1 and W1.2
immediately after (contained, high confidence, measurable) → W2 as a
6.0-beta bytecode change with the counter as evidence → W3 prototyped
behind the flag in parallel, promoted only if the dashboards agree →
W4 as documentation/tooling work anyone can pick up independently.
Nothing here blocks beta-1; W2 is the only one that wants to land
before a format freeze.
The bytecode change is not necessarily deeply impacting in terms of
compatibility. We can use a different bootstrap method for this and keep
the old one in parallel for example.
[...]
I've read both PRs in depth. Here's the assessment and how they slot
into the roadmap.
## PR 2549 — Jochen's GROOVY-12023 indy cache rework
**What it actually does** (the title undersells it — this is a
redesign of the call-site lifecycle):
1. **A real PIC chain in the call-site target.** Up to 4
(`groovy.indy.pic.size`) class-guarded handles chained via
`guardWithTest` and installed with `setTarget`, with **one** top-level
SwitchPoint guard around the whole chain (each link is built with
`skipSwitchPoint = true`). On master, only a *monomorphic* winner ever
gets `setTarget`; polymorphic sites bounce through the folded
`exactInvoker` path forever. This is what produces the 7.06× on
`dispatch_3_polymorphic_groovy` and 2.35× on megamorphic in the PR's
benchmark run — and unlike PR 2591's alert (see below), those deltas
are credible because they're exactly what a PIC chain should deliver
and the alert lists only Groovy benchmarks.
https://github.com/apache/groovy/pull/2549#pullrequestreview-4414324894
I am confused... I did before see only ms/op. And this is ops/ms. I did
go through the edits before and they are all ms/op... no.. actually some
are op/ms and other are ms/op. But whatever compares does not care about
that and only thinks in higher=worse. Can we improve that? Like have all
benchmarks uniformly enter either ms/op or ops/ms and then compare with
less is better for ms/op or more is better for ops/ms.
The branch itself is bit of a mess, but some of the ideas in there I
have transformed already in other PRs, for example the DGM MethodHandles
and the DTT improvement. There is more to take from. The leading idea
was a simple PIC in Methodhandles, to try to stabilize the target. Then
moving guards from the branches to the front of the PIC. The switchpoint
guard for example, and removing the switchpoint guard if not needed. But
to do those things cleanly I think we need something like a "profile" on
the callsite, information we keep and can reuse later for extending the
handle.
But I genuinely missed the performance increase reports
2. **Lock-free MRU entry + identity keys.** The synchronized LRU map
moves to level 3; the common repeat-hit goes through a volatile
single-entry check. Cache keys become `receiver.getClass()` /
`ClassValue` objects instead of concatenated class-name Strings —
killing per-call String allocation and hashing in the cold tier.
I think that is something to take over
3. **Degraded mode for metaclass churn.** After 10 fallback rounds,
the site abandons the SwitchPoint and installs a class-guarded handle
dispatching through `InvokerHelper.getMetaClass().invokeMethod()`.
This is significant beyond Grails-churn relief: **`invokeDegraded` is
the first working instance of "a MethodHandle calling the
MOP/reflection" from his mail** — the exact building block workstream
W3 (reflective cold tier) needs, just triggered by churn instead of
coldness.
The idea is to start on a cold path, optimize the hot path and fall back
to a slow path if the callsite degrades. The slow path and the cold path
could very well be the same. But they don´t have to. The work in this PR
has been done before I was looking at the cold paths. Which does not
deny that there are possible overlaps.
4. **Cold-path trims that overlap my W1.** `catchException` becomes
conditional (only GroovyObject MOP methods — resolving the old "TODO:
save this guard" in `Selector`), the boot handle binds 4 arguments
instead of 8 (flags move into `CacheableCallSite`), wrappers carry
their SwitchPoint so stale entries self-evict, the polling
cache-cleaner becomes a proper `ReferenceQueue`, and there's an MRU
classloader-leak guard. Plus `DefaultTypeTransformation` hot/cold
method splitting for inline-budget friendliness and a cleaner
`findStatic`-in-`<clinit>` DGM `TARGET`.
looks like I already forgot a few things I put into those spikes, haha.
**Concerns to raise in review:**
- `invokeDegraded`/`getPropertyDegraded` **return** the caught
`GroovyRuntimeException` as the call result (`return e`). The comment
claims parity with `catchException + UNWRAP_EXCEPTION`, but master's
unwrap rethrows the cause — returning the exception object to the
caller would be a semantic change. Worth a targeted test (MOP methods
using exceptions for control flow, e.g. `MissingMethodException`
fallthrough).
yeah.. there was weirdness with ScriptToTreeNodeAdapterTest especially.
And I do mean it expecting a returned exception somehow. One of the
reasons why I wanted to do the changes from this branch more isolated
and incremental
- In degraded mode, `fromCacheHandle` rebuilds the degraded handle
(plus `asSpreader`/`asType`) on every invocation that reaches it, with
no caching; and polymorphic degraded sites `setTarget` per
receiver-class change. Probably rare in practice, but it's per-call MH
construction on a path chosen *because* the site is unstable.
yeah, that sounds bad.
- `picInsertIfMissing` doesn't insert anything — it's a containment
check; the name will mislead maintainers.
got messed up over time... again, that is why I want to restart on this.
- It's ~73+ commits behind master and will conflict with GROOVY-12092
(the cleaner-thread flag touches code this PR replaces). The 4 failing
JDK-25 tests are `ListMultimap`/Guava resolution failures in the test
environment — almost certainly unrelated flakes, but should be
confirmed on rebase.
I would not rebase it. Instead take the idea and do a new cleaner
implementation in its own PR.
**Verdict:** this is the highest-value open work on the cold/dispatch
problem and substantially delivers workstream W1 while adding a
hot-path polymorphic win my plan didn't claim. Path to merge: rebase,
describe the PR (body is empty), address the exception-semantics
question, rerun full suites + the cold bench. Importantly, it does
**not** cover: per-`CachedMethod` unreflect caching
(`Selector.setBaseHandleForCachedMethod` still unreflects on every
miss), MethodType erasure (W2), a coldness-triggered reflective tier
(W3 proper), or AOT (W4) — so the roadmap stands, with W3 now best
framed as *generalizing this PR's degraded-mode machinery*: use an
`invokeDegraded`-style MOP handle as every site's initial cold target,
and promote to selector-built handles at threshold.
>
The question for me is more how much of this should be done before beta1
## PR 2591 — Daniel's GROOVY-12065 peephole optimizer
A 737-line `PeepholeOptimizingMethodVisitor` wired into
`AsmClassGenerator` that compacts constant loads (`LDC` →
`ICONST_*`/`BIPUSH`/`SIPUSH`/`LCONST_*`/`FCONST_*`/`DCONST_*`,
correctly preserving `-0.0f`/`-0.0d`), and simplifies `OperandStack`
by routing constants through `visitLdcInsn` (−51 lines). Tests pass,
coverage 94%, review comments are minor (Copilot's `visitCode` pairing
nit; Eric's operand-stack-type question answered by Jochen — ASM
handles it).
**Honest sizing of the win:** compacted constants shrink method
bytecode, and HotSpot's inlining thresholds (`MaxInlineSize`=35
bytecode bytes) count bytes — so this genuinely helps small generated
methods near inline cliffs, plus class-file size. But it's a marginal
steady-state win, not a dispatch fix. The PR's benchmark alert
claiming 1.6–5.4× improvements is **provably runner noise**: pure-Java
baselines (`dispatch_8_megamorphic_java` 5.37×, `staticFib_java`
1.90×) "improved" too, which a Groovy codegen change cannot do.
and that is a clear sign of polluting the context ;) Daniel´s PR has
nothing to do with cold path improvements.
**Verdict:** low-risk, merge-worthy after the minor comments; its real
strategic value is the *infrastructure* — a peephole pass is where
future wins live (dead-store elimination, box/unbox pair elimination,
`DUP`/`POP` cleanup). Suggest adding a deterministic bytecode-size
metric (total bytes for a fixed compile corpus) to the compiler
dashboard so this class of change gets a noise-free signal.
here I agree
## One cross-cutting finding
Both PRs' comment threads expose the same measurement problem: the
per-PR JMH comparison fetches gh-pages history from *different runner
hardware*, so it flagged impossible 2–5× swings on pure-Java
benchmarks. That makes per-PR benchmark gating actively misleading
today. Cheap fix that strengthens W0: have the PR workflow build and
run **master HEAD and the PR head in the same job on the same runner**
and compare within-run, instead of comparing against historical
`data.js` — plus the LambdaForm-spin counter, which would have cleanly
quantified 2549's cold-path effect and shown 2591 as neutral.
My idea would be to have a baseline benchmark in Groovy unrelated code
in pure Java. And then adjust everything to be relative to that. The AI
comment makes the assumption that a runner is performing the same
throughout the benchmark suits. I do not know about that. It surely is
not the case on my local computer. But my idea also would suffer from this.
[...]
bye Jochen