[
https://issues.apache.org/jira/browse/GROOVY-12191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100604#comment-18100604
]
ASF GitHub Bot commented on GROOVY-12191:
-----------------------------------------
paulk-asert commented on PR #2736:
URL: https://github.com/apache/groovy/pull/2736#issuecomment-5137823765
AI read:
> @blackdrag asked how often the ancestor-metaclass pattern appears in
Grails. I looked at that — and, since the answer only matters if hierarchy
fan-out is load-bearing in the first place, I also tested that premise
empirically on this PR's head (`637bdaf`). Three parts, then a suggested way
forward.
>
> ## 1. Direct answers to the two open questions
>
> **"Are we even caching in that case? If we are then caching that would be
new in this pull request I assume."**
>
> Caching is pre-existing, but the cached thing is not a frozen miss. When
selection finds no method, `Selector.setMetaClassCallHandleIfNeeded` (unchanged
by this PR, `cache = true`) links a route through `MetaClass.invokeMethod`
bound to the receiver's MetaClass instance. Each invocation of that linked
handle re-runs `invokeMissingMethod` → `findMethodInClassHierarchy`
(`MetaClassImpl.java:950`), and the walk resolves ancestor MetaClasses
**freshly from the registry on every call** (`MetaClassImpl.java:3949`).
>
> **"I simply think … a simple uncached path through the metaclass would be
sufficient."**
>
> It's stronger than that: the cached route already *is* a live path. A
linked "miss" site observes ancestor EMC changes with **no invalidation at
all**.
>
> ## 2. Experiment: disable fan-out, test behaviour only
>
> Since the indy `Selector` consults only the receiver's own MetaClass
tables at link time, I reduced `invalidateClassHierarchy` to root-only (no
descendant walk) on the PR head and ran behaviour-only probes through hot call
sites (200 iterations to ensure linkage) — no `hasBeenInvalidated()`
assertions, only observable dispatch results:
>
> | Probe | fan-out OFF | fan-out ON |
> |---|---|---|
> | linked miss → parent EMC adds method (case 4 in the matrix above) | ✅ |
✅ |
> | linked miss on `String[]` → `Object[]` EMC adds method | ✅ | ✅ |
> | present child method vs parent adds more-specific overload | ✅ | ✅ |
> | linked hit via snapshot → parent EMC removed (expecting miss) | ❌ stale
| ❌ stale — **identical** |
>
> Observations:
>
> - The canonical "case that *does* need hierarchy SP" passes **without** it
— the dynamic route self-heals, including across the array lattice.
> - The one failing probe fails identically in both configurations: the
child MetaClass was constructed after the parent EMC add, so the method sits in
the child's own tables (construction-time snapshot). Re-linking re-selects from
those unchanged tables — no SwitchPoint policy can alter the outcome. This is
the same behaviour as @blackdrag's D/E and F/G scripts.
> - The 19 existing tests that fail with fan-out disabled all assert
**SwitchPoint retirement** (mechanism), not dispatch results.
>
> Why this holds generally: ancestor-dependent resolution reaches a linked
site in exactly two ways — the dynamic `invokeMethod` route (live per call) or
methods copied into the receiver's own MetaClass (snapshot / EMC inheritance,
where any refresh fires the *child's* own `incVersion`). Re-linking against an
unchanged receiver MetaClass reproduces the same target, so retiring subtype
SwitchPoints when only an ancestor changed appears **behaviourally inert** for
stock `MetaClassImpl`/EMC.
>
> Caveats, honestly stated: I probed method dispatch, not property sites;
and a custom MetaClass implementation that consults ancestor state during
*selection* could still go stale — but that unknown-MC case could be served by
bulk invalidation (rare event) rather than an indexed subtype walk. If there's
a counter-example where fan-out changes observable behaviour for stock
metaclasses, that would be the decisive artifact.
>
> <details>
> <summary>Probe test source (drop into <code>src/test/groovy/bugs/</code>
to reproduce)</summary>
>
> ```groovy
> package bugs
>
> import org.junit.jupiter.api.AfterEach
> import org.junit.jupiter.api.Test
>
> final class FanOutBehaviourExperiment {
>
> static class Parent {}
> static class Child extends Parent {}
> static class Base2 {}
> static class Sub2 extends Base2 {}
>
> @AfterEach
> void tearDown() {
> [Parent, Child, Base2, Sub2, Object[].class, String[].class].each {
> GroovySystem.metaClassRegistry.removeMetaClass(it)
> }
> }
>
> private static String probe(x) {
> try { return x.onlyOnParent() } catch (MissingMethodException e) {
return 'miss' }
> }
>
> @Test
> void linkedMissThenParentEmcAdd() {
> def c = new Child()
> 200.times { assert probe(c) == 'miss' } // hot-link the miss
route on Child
> Parent.metaClass.onlyOnParent = { -> 'now-visible' }
> assert probe(c) == 'now-visible' // passes with fan-out
disabled
> }
>
> private static String probe2(x) {
> try { return x.onlyOnBase() } catch (MissingMethodException e) {
return 'miss' }
> }
>
> @Test
> void linkedHitViaParentEmcThenRemove() {
> Base2.metaClass.onlyOnBase = { -> 'from-base' }
> def s = new Sub2() // Sub2 MC created
AFTER add → snapshot
> 200.times { assert probe2(s) == 'from-base' }
> GroovySystem.metaClassRegistry.removeMetaClass(Base2)
> assert probe2(s) == 'miss' // fails IDENTICALLY
with and without fan-out
> }
>
> private static String probeArr(x) {
> try { return x.arrHello() } catch (MissingMethodException e) {
return 'miss' }
> }
>
> @Test
> void linkedMissThenObjectArrayEmcAdd() {
> String[] arr = ['a', 'b']
> 200.times { assert probeArr(arr) == 'miss' }
> Object[].metaClass.arrHello = { -> 'array-visible' }
> assert probeArr(arr) == 'array-visible' // passes with fan-out
disabled
> }
> }
> ```
> </details>
>
> ## 3. Grails (the direct question)
>
> I audited `apache/grails-core` `8.0.x` (the monorepo — includes GORM, GSP,
testing-support; 87 modules).
>
> **How often does ancestor-EMC visibility occur? It's structural, not
rare:**
>
> - The codec system registers every codec's `encodeAsX`/`decodeX` onto the
EMCs of `[String, GStringImpl, StringBuffer, StringBuilder, Object]`
(`CodecMetaClassSupport.resolveDefaultMetaClasses()`). Every
`someValue.encodeAsHTML()` on an arbitrary receiver is exactly the
`Object`-ancestor pattern — registered at startup
(`DefaultCodecLookup.registerCodecs`) and per test class
(`GrailsWebUnitTest.mockCodec`).
> - Dev reload clears the MetaClasses of 15 common classes including
`Number` (`DefaultGrailsPluginManager.COMMON_CLASSES`).
>
> **But** these events fire at startup / reload / test-setup, and both
resolution modes they produce (construction snapshot; dynamic walk) behave
identically with fan-out disabled per §2. Also relevant: global EMC mode is
**off** in the modern runtime (`enableGlobally()` appears only in legacy
test-support), and GORM 8 is trait-based — `GormEnhancer` does runtime EMC
registration only for Java entities or explicit `dynamicEnhance`.
>
> **Where Grails hurts today is what the scoping *core* fixes:**
>
> - GSP tag dispatch registers tag methods into page/taglib/controller EMCs
on **first use of each tag per class, in production, at request time**
(`TagLibraryMetaUtils.methodMissingForTagLib`, `addMethodsToMetaClass =
!developmentMode`). A warming-up app fires many registry events while serving
traffic — each one a **process-wide** deopt under the current global
SwitchPoint, scoped to one leaf class under this PR.
> - Dev reload (`removeMetaClass` + fresh EMC per changed class, GSP
recompile) and test suites (`mockCodec` per test class, GORM `close()` per
entity) churn metaclasses constantly.
>
> That is exactly the shape of the CI JMH grails-suite result on this PR
(≈5.7× calibrated; `burstThenSteadyState` ≈112×) — and none of it depends on
hierarchy fan-out, which only ever *adds* invalidations.
>
> ## Suggested way forward
>
> - **Keep** the per-MetaClass SwitchPoint scoping core — that's the
measurable win, and Grails is its best witness.
> - **Drop** `ClassHierarchyIndex` and the hierarchy fan-out policy for
stock `MetaClassImpl`/EMC (with the mechanism tests that assert fan-out), per
@blackdrag's argument, which the experiment supports.
> - For unknown/custom MetaClass kinds, stay correctness-first via **bulk**
invalidation on those rare events instead of an indexed subtype walk.
> - Property-site probes analogous to §2 would be worth adding before merge
to close the remaining coverage gap.
>
> That's a strictly smaller machine with, as far as these experiments can
see, identical user-visible semantics — and it keeps the full performance story
for beta-1.
> Scope indy SwitchPoint invalidation
> -----------------------------------
>
> Key: GROOVY-12191
> URL: https://issues.apache.org/jira/browse/GROOVY-12191
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
> Fix For: 6.0.0-beta-1
>
>
> h3. Problem
> With invokedynamic enabled (default since Groovy 4), linked MOP call sites
> were guarded by a *single process-wide* {{SwitchPoint}}
> ({{{}IndyInterface.switchPoint{}}}).
> Any MetaClass registry change or category enter/leave invalidated that switch
> point, so *every* linked site fell back and re-linked — including sites whose
> receiver type was unrelated.
> That global invalidation is expensive when MetaClass churn is common (e.g.
> ExpandoMetaClass / mixins on startup or per-request paths, Grails-like
> patterns). Unrelated hot monomorphic sites pay re-link and JIT deopt cost
> they should not.
> h3. Goal
> Keep linked call sites optimized unless the *relevant* MetaClass state for
> that site actually changed.
> h3. Approach
> One SwitchPoint domain {*}per class{*}, stored on {{{}ClassInfo{}}}:
> * MetaClass change for type {{T}} retires {{{}T{}}}'s SwitchPoint *and*
> those of loaded subtypes / implementors (hierarchy fan-out).
> * Unrelated types keep their SwitchPoints; their call sites stay optimized.
> * Category enter/leave (and {{{}VMPlugin.invalidateCallSites(){}}})
> bulk-retire *all* loaded class SwitchPoints so sites re-link under the new
> category state. There is *no* second category SwitchPoint on the hot path.
> * Linked handles always install a *single* class-domain guard via
> {{IndyInvalidation.guardWithMopSwitchPoints(...)}} — same monomorphic guard
> shape as before, without global deopt on unrelated MetaClass churn.
> Final classes short-circuit hierarchy fan-out (no full {{ClassInfo}} scan).
> Non-final types batch retirements with {{{}SwitchPoint.invalidateAll{}}}.
> h3. Invalidation map
> ||Event||What is retired||
> |MetaClass change for type {{T}} (registry /
> {{{}ClassInfo.incVersion{}}})|{{T}} + loaded subtypes / implementors|
> |Category enter/leave, {{invalidateCallSites()}}|All loaded class
> SwitchPoints (bulk)|
> |Unattributed MetaClass registry event|All loaded class SwitchPoints (bulk)|
> |First MetaClass *install* on a class|Version bump only (no linked sites
> yet); replacement / clear retires that class's SwitchPoint|
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
