[
https://issues.apache.org/jira/browse/GROOVY-12191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18099769#comment-18099769
]
ASF GitHub Bot commented on GROOVY-12191:
-----------------------------------------
daniellansun commented on PR #2736:
URL: https://github.com/apache/groovy/pull/2736#issuecomment-5106124863
@blackdrag Thank you for pressing on the “when / why” of hierarchy fan-out.
That is the right question, and the previous reply was still too coarse. Below
is the refined model after re-checking MOP selection against your scenario.
## 1. When we invalidate (and when we do not)
Monomorphic indy sites guard a **class-domain** SwitchPoint on the
*receiver’s* runtime class (via `ClassInfo`). They re-link only when *that*
domain is retired.
For your ladder `A extends B extends C` and `call(C x) { x.fooC() }` with `c
= new C()`:
| Operation | Retires `C`’s domain? | `foo*` visible on `c` / `C.metaClass`?
|
|-----------|----------------------|----------------------------------------|
| `B.metaClass = EMC` + `fooB0` | **No** (`C` is a *supertype* of `B`) |
**No** |
| `B.metaClass = null` | **No** | **No** |
| `enableEMCglobally()` | Only if a bulk path fires; not via typed `B`
change | Global EMC changes *creation*, not `C`’s linked target by itself |
| `c.metaClass = null` | Per-instance path → exact `C` only if a class-level
SP was tied to that path | Restores class-level MC for `c` |
| `B.metaClass = new MetaClassImpl()` | **No** for pure `MetaClassImpl`
(exact `B` only) | **No** |
| Detached EMC `fooB1` after replace | Depends on whether that EMC is still
the registered MC | **No** on `c` |
| `A.fooA = {}` | **No** for `C` | **No** on `c` |
| Per-instance EMC on `b` + `fooB2` | **No** for `C` | **No** on `c` |
So for `call(c)` / `fooC`, the new scoped model already keeps `C`’s site
linked across the operations that only touch `B`/`A`. That is the main
behavioural difference vs the old process-wide SwitchPoint (which retired
*everything*).
Regression coverage for this matrix is in
`Groovy12191.blackdragScenario_callOnC_notInvalidatedBySubclassMetaClassChurn`.
## 2. Why hierarchy exists at all (and when it does not)
Hierarchy fan-out is **not** “`MetaClassImpl` shares one table up the
hierarchy”. Each class still has its own MetaClass / `ClassInfo` domain.
It exists because **selection** can observe *ancestor* MetaClass state:
- `MetaClassImpl.findMethodInClassHierarchy` only opens a real walk when
some strong MetaClass in the hierarchy is a **modified** `MutableMetaClass`
(EMC is the common case).
- Empirically, `Parent.metaClass.hello = {…}` is visible on `new
Child().hello()` **without** `enableGlobally()`.
- Empirically, `Object[].metaClass.arrHello = {…}` is visible on `String[]`
— that is why the array lattice is indexed.
So hierarchy is required for **cross-class MOP visibility**, not for
ordinary `MetaClassImpl` method tables.
### Refined policy (exact-class is an allow-list)
| Event | Width |
|-------|--------|
| EMC install / remove / in-place update (`incVersion`) | class + loaded
subtypes |
| Global EMC creation handle active | class + hierarchy |
| Interface or array MetaClass change | class + hierarchy (array lattice) |
| Modified custom `MetaClassImpl` subclass (`isModified()==true`) | class +
hierarchy |
| Pure unmodified `MetaClassImpl` ↔ `MetaClassImpl` (or null) | **exact
class only** |
| Per-instance MetaClass change | **exact class only** |
| Category enter/leave, unscoped registry event | all loaded class domains |
This matches your intuition: **without EMC (and without a modified mutable
MetaClass), hierarchy is unnecessary**. Pure `MetaClassImpl` parent replace no
longer fans out to children. Unknown/custom MetaClass kinds still fan out
(correctness-first).
## 3. “Should the MetaClass own the SwitchPoint?”
Agreed in principle. The current class-domain SP is a **stand-in for
class-level MetaClass generation**, not a claim that every MetaClass kind
shares the same invalidation logic.
A MetaClass-owned guard would be the natural place for:
- MetaClass-specific update vs replace semantics,
- true per-instance domains (today: uncacheable PIC + exact class retire),
- custom MetaClass implementations that do not fit EMC / `MetaClassImpl`.
That is a larger redesign than this PR. For 6.0 we kept a single monomorphic
hot-path guard and made invalidation **MetaClass-aware** at the registry
boundary. MetaClass-owned SwitchPoints remain a follow-up once the “when / why”
matrix above is agreed.
## 4. Update vs replace, and `version`
You are right that parent method *addition* is often an **update** of the
same MetaClass, not a registry replace. That path is `ClassInfo.incVersion()`
(from EMC), which still does hierarchy fan-out — because subtype sites may have
linked a miss (or an older target) and must re-select to see the new expando
method.
Per-instance MetaClass continues to use the existing uncacheable PIC path
(`canSetTarget == false`); we do not introduce per-instance SwitchPoints in
this PR.
## 5. PIC sentinel rename
Per
[discussion_r3664025383](https://github.com/apache/groovy/pull/2736#discussion_r3664025383):
- `NULL_METHOD_HANDLE_WRAPPER` → `UNCACHEABLE_PIC_SENTINEL`
- meaning: “do not store this receiver shape under a class-keyed PIC entry”
- deprecated alias kept as `getNullMethodHandleWrapper()` for in-tree
callers during the rename
## 6. What we are *not* claiming
- We do **not** claim minimal invalidation at method-name granularity
(`fooC` unchanged ⇒ never re-link). The monomorphic SwitchPoint still retires
the whole class domain when that class’s MOP generation changes — same shape as
pre-6.0, but **scoped**.
- We do **not** claim MetaClass-owned SwitchPoints are done; only that
policy is now MetaClass-aware on top of class domains.
Happy to adjust further if you want the exact-only allow-list tightened or
loosened (e.g. EMC-only fan-out even for interfaces/arrays without EMC).
> 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)
