[
https://issues.apache.org/jira/browse/GROOVY-12191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100334#comment-18100334
]
ASF GitHub Bot commented on GROOVY-12191:
-----------------------------------------
blackdrag commented on PR #2736:
URL: https://github.com/apache/groovy/pull/2736#issuecomment-5128475894
[...]
> Agreed on the important refinement:
>
> * `MetaClassImpl.findMethodInClassHierarchy` is a **missing-method**
path. It only opens a real walk when some strong MetaClass in the hierarchy is
a **modified** `MutableMetaClass` (EMC is the usual case).
which means we add a lot of machinery for a small feature.
```
class A {}
class B extends A {}
class C extends B {}
def call2(x,y){x.m2(y)}
def b = new B()
A.metaClass.m2 = {x -> "A3"}
assert call2(b,0) == "A3" // m2 discovered by findMethodInClassHierarchy
A.metaClass.m2 = {Integer x -> "A4"}
assert call2(b,0) == "A4" // Mutation of parent. More specific m2 discovered
by findMethodInClassHierarchy?
B.metaClass.m2 = {x -> "B3"}
assert call2(b,0) == "B3" // B.m2 *blocks* findMethodInClassHierarchy, more
specific method on A ignored!
assert call2(new C(),0) == "A4" // B.m2 does *not block*
findMethodInClassHierarchy.
```
Sure, considering how this is implemented it makes sense, but from a user
side I think it does not.
> * A method that already exists on the child keeps winning for
applicable arguments after hierarchy re-link; SwitchPoint retirement does
**not** rebuild MetaClass tables.
> * The **C vs D** divergence (same structure, different MetaClass
construction time relative to parent EMC) is pre-existing MOP behaviour:
construction-time snapshot / inheritance of ancestor expando methods into a new
`MetaClassImpl`, not something class-domain SwitchPoints can honestly “fix”.
[...]
> Hierarchy SwitchPoint is only there so **already-linked miss** sites on
subtypes re-select when an ancestor becomes / stops being a modified mutable
MetaClass (e.g. linked miss → parent adds method; linked hit → parent EMC
removed).
So if class A extends B and A or B have a method foo, and we have a callsite
new B().foo(x), then we do not need to invalidate it if the meta class of A
changes. Only if the metaClass for A changes (new or mutates) then we have to
invalidate the callsite. If this is the case, then why the hierarchy logic? If
I take the above together I get for example something like this on a pure meta
class level:
```
class D{def foo(x){1}}
class E extends D{}
assert new E().foo(0) == 1
D.metaClass.foo = {Integer x -> 2}
assert new E().foo(0) == 1
registry.removeMetaClass(E)
assert new E().foo(0) == 2
registry.removeMetaClass(D)
assert new E().foo(0) == 2
```
Only replacing the meta class of E made the more specific foo in D visible
and it stays visible, even though we removed the meta class of D. Is it
because of the declared method?
```
class F {}
class G extends F {}
F.metaClass.foo = {Integer x -> 1}
F.metaClass.bar = {Integer x -> -1}
assert new F().foo(0) == 1
assert new G().foo(0) == 1 // F.foo visible
G.metaClass.foo = {x -> 2}
assert new G().foo(0) == 1 // F.foo more specific
registry.removeMetaClass(F)
assert new G().foo(0) == 1 // F meta class replaced, EMC(G) still sees the
old F.foo, no switch to G.foo
F.metaClass.foo = {Integer x -> 10}
F.metaClass.bar = {Integer x -> -10}
G.metaClass.bar = {x -> -20}
assert new G().foo(0) == 1 // new EMC(F) with new foo(Integer) method, still
no change for EMC(G)
assert new G().bar(0) == -1 // bar invoked first time, still sees the old
bar method only.
```
No it is not. To sum it up: In what case *must* the mutation/replacement of
a parent meta class invalidate a callsite if the meta class of the current
receiver stays unchanged?
My goal is not to define new meta class logic, my goal is to find out for
what that hierarchy switch point logic is actually needed. It is not clear to
me from the code. So what exactly is it trying to solve?
> 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)
