daniellansun commented on PR #2736:
URL: https://github.com/apache/groovy/pull/2736#issuecomment-5082859135

   # GROOVY-12191 Performance Verification Report V2
   
   **Scoped indy SwitchPoint invalidation** (re-verification after review 
follow-ups)
   
   | Item | Value |
   |---|---|
   | Issue | [GROOVY-12191](https://issues.apache.org/jira/browse/GROOVY-12191) 
|
   | Commit under test | `f48ab19256f6ae9888dd48bb34cd770dd6a05958` |
   | Baseline (parent) | `0ca665dad0d4070bfa218d9b5577ec423c8b2181` |
   | Date | 2026-07-26 |
   | Verdict | **PASS** — cross-type MetaClass churn no longer globally 
deoptimizes hot monomorphic call sites; steady-state hot path is parity; 
hierarchy fan-out is indexed; correctness suite green (76/76) |
   
   ---
   
   ## 1. Executive summary
   
   This verification covers the **full GROOVY-12191 stack** on the current tip 
of PR #2736:
   
   1. **`ae8e497`** — Replace the process-wide MOP `SwitchPoint` with a 
**per-class domain** on `ClassInfo` (plus hierarchy fan-out). Category 
enter/leave and unscoped MetaClass events still bulk-retire loaded class 
domains. Linked call sites still carry **one** SwitchPoint guard on the hot 
path.
   2. **`f48ab19`** — Address review: restore synchronized legacy `switchPoint` 
rotation; document `incVersion()` / field semantics; fix array-class fan-out 
via a weak **ancestor→descendant index** (`ClassHierarchyIndex`); typed 
MetaClass invalidation becomes `O(|subtypes of T|)` instead of scanning every 
loaded `ClassInfo`.
   
   On the same host, JDK, and JMH annotation settings, A/B measurement of 
**HEAD (`f48ab19`) vs baseline (`0ca665da`)** shows:
   
   1. **Near-zero regression on steady-state monomorphic hot paths** (baseline 
ratios ≈ 0.96–1.10×, within noise).
   2. **Large gains under cross-type MetaClass churn** — the primary win:
      - `CallSiteInvalidationBench.crossTypeInvalidationEvery1000`: **428.4 → 
2.25 ms/op** (≈ **190×**).
      - `ScopedInvalidationBench.hotLoop_unrelatedMetaClassChurn`: throughput ≈ 
**197×**.
   3. **Framework-style “unrelated burst then steady request loop”** 
(`burstThenSteadyState`): ≈ **114×**.
   4. **Same-type invalidation and Category semantics are not “falsely 
optimized”**: same-type churn remains far slower than baseline (forced 
re-link); category enter/leave stays bulk and matches parent throughput (~1×).
   5. **Correctness: 76/76 tests passed** (full scoped-invalidation + 
hierarchy-index suite).
   
   > In one line: without adding a second hot-path guard, the change shrinks 
the blast radius of unrelated MetaClass mutations from process-global to 
type-scoped (with correct hierarchy/array fan-out), with measured gains on the 
order of **10¹–10²×** (depending on churn frequency and site count).
   
   ---
   
   ## 2. Change mechanism and performance hypotheses
   
   ### 2.1 Previous model (baseline `0ca665da`)
   
   * `IndyInterface.switchPoint` was a **process-wide shared** `SwitchPoint`.
   * Any MetaClass registry change → `invalidateSwitchPoints()` → **every** 
linked indy site’s guard failed → mass re-link.
   * Typical symptom: a framework/plugin extends MetaClass on *ColdType* at 
startup or hot-reload; application hot paths on *HotTarget.compute()* were 
deoptimized together with it.
   
   ```text
   // baseline (simplified)
   protected static SwitchPoint switchPoint = new SwitchPoint();
   // MetaClassRegistry listener → always:
   synchronized (IndyInterface.class) {
       SwitchPoint old = switchPoint;
       switchPoint = new SwitchPoint();
       SwitchPoint.invalidateAll(new SwitchPoint[]{old});
   }
   ```
   
   ### 2.2 New model (`ae8e497` + `f48ab19`)
   
   | Component | Role |
   |---|---|
   | `SwitchPointInvalidator` | Single-domain lifecycle: CAS `get` / 
`detachLive` / `invalidate` |
   | `IndyInvalidation` | Policy: `invalidateClass` (+ hierarchy), 
`invalidateCategory` (bulk), `guardWithMopSwitchPoints` |
   | `ClassHierarchyIndex` | Weak ancestor→descendant index; O(\|subtypes\|) 
fan-out incl. JLS array lattice |
   | `ClassInfo` | Holds one `indySwitchPoint` per class; registers into the 
index; coordinates version bumps |
   | `Selector` / `IndyCompoundAssign` / `ColdReflectiveMethodHandleWrapper` | 
Install a **per-receiver-class** SwitchPoint guard at link time |
   
   **Hot-path shape (intentionally monomorphic):**
   
   ```text
   handle = classSp.guardWithTest(fast, fallback)
   ```
   
   Still a single guard—matching the pre-6.0 monomorphic shape. There is **no** 
second category SwitchPoint on the hot path.
   
   **Invalidation policy:**
   
   | Event | Behavior | Expected hot-path cost |
   |---|---|---|
   | Unrelated-type MetaClass change | Retire that type’s domain (and indexed 
subtypes) only | Hot sites **do not** re-link |
   | Same-type / parent MetaClass change | Retire class + hierarchy fan-out | 
Must re-link (correctness) |
   | Category enter/leave, `invalidateCallSites()` | Bulk-retire all loaded 
class domains | Same order of cost as old global invalidation |
   | Unattributed MetaClass event | `invalidateUnscoped()` bulk | Conservative, 
correctness-first |
   | Array root (`Object[]`, interface arrays) | Fan-out via full JLS array 
covariance lattice in the index | Correct subtype retirement (review fix) |
   
   **What `f48ab19` specifically adds beyond `ae8e497`:**
   
   | Review item | Resolution in `f48ab19` |
   |---|---|
   | Array fan-out hole (`Object[]` treated as final leaf) | 
`ClassHierarchyIndex` indexes full array lattice |
   | Legacy `switchPoint` lost lock | Restored `synchronized 
(IndyInterface.class)` on bulk rotation |
   | `switchPoint` semantics over-claim | Docs: bulk-only rotation; 
`@Deprecated(since="6.0.0", forRemoval=false)` |
   | `incVersion()` no longer global flush | Documented; bulk callers → 
`invalidateCallSites()` / `invalidateCategory()` |
   | Scalability O(N×M) per typed MC change | Weak subtype index → O(\|subtypes 
of T\|); category still full walk |
   
   ### 2.3 Hypotheses under test
   
   | ID | Hypothesis | How verified |
   |---|---|---|
   | H1 | Baseline hot loop has no material regression | A/B ratio ≈ 1 |
   | H2 | Cross-type churn throughput/latency improves substantially | 
Unrelated ≫ parent; on HEAD, unrelated ≫ same-type |
   | H3 | Pure hot loop after unrelated burst approaches baseline | 
`afterUnrelatedBurst` ≈ `baseline` |
   | H4 | Same-type / hierarchy / category still pay re-link tax | Clearly 
slower than baseline |
   | H5 | Correctness suite stays green (incl. index / array cases) | Unit / 
integration tests |
   
   ---
   
   ## 3. Methodology
   
   ### 3.1 Environment
   
   | Item | Value |
   |---|---|
   | OS | Linux 6.15.5 x86_64 (hostname: `hera`) |
   | CPU | AMD EPYC 7763 (6 vCPUs visible) |
   | Memory | 23 GiB |
   | JDK | Amazon Corretto **25.0.2** (`25.0.2+10-LTS`) |
   | Build | Gradle wrapper, `./gradlew :perf:jmh`, indy enabled by default |
   | Trees | HEAD: project worktree at `f48ab19`; baseline: worktree at 
`0ca665da` (`/tmp/groovy-12191-parent`) |
   | Bench source parity | `ScopedInvalidationBench` + 
`CallSiteInvalidationBench` workloads identical on parent (sources copied into 
parent worktree for fair A/B) |
   | Run order | Sequential (HEAD then parent per suite) to avoid dual-JVM core 
contention; no CPU pinning |
   
   ### 3.2 Benchmark suites
   
   1. **`org.apache.groovy.bench.ScopedInvalidationBench`** (added by 
`ae8e497`; Throughput, ops/ms)
      - 100 000 monomorphic calls per op; MetaClass write every 1000 iterations 
in churn cases.
      - JMH: `@Warmup(3×2s) @Measurement(5×2s) @Fork(2)` → 10 samples per 
benchmark.
   
   2. **`org.apache.groovy.perf.grails.CallSiteInvalidationBench`** (existing 
Grails-oriented suite; AverageTime, ms/op)
      - Same 100 000-iteration loops; cross-type / same-type / burst patterns.
      - Same JMH annotation settings.
   
   ### 3.3 Correctness
   
   ```bash
   ./gradlew :test --tests Groovy12191 \
     --tests org.apache.groovy.runtime.indy.IndyInvalidationTest \
     --tests org.apache.groovy.runtime.indy.SwitchPointInvalidatorTest \
     --tests org.apache.groovy.runtime.indy.ClassHierarchyIndexTest \
     --tests org.codehaus.groovy.vmplugin.v8.IndyScopedSwitchPointTest 
--rerun-tasks
   ```
   
   **Result: 76 passed / 0 failed / 0 skipped / 0 error.**
   
   | Test class | Cases (approx.) | Notes |
   |---|---:|---|
   | `bugs.Groovy12191` | 10 | End-to-end MOP / category / `incVersion` scoping 
|
   | `IndyInvalidationTest` | 34 | Policy, bulk, unscoped, hierarchy, 
concurrency probes |
   | `SwitchPointInvalidatorTest` | 12 | CAS lifecycle, concurrent detach/get 
races |
   | `ClassHierarchyIndexTest` | 9 | Class/interface/array lattice indexing 
(**f48ab19**) |
   | `IndyScopedSwitchPointTest` | 11 | Guard install, cold tier, logging child 
process |
   
   ### 3.4 Statistics and artifacts
   
   * Scores are JMH means; `±` is JMH’s default **99.9% CI**.
   * **Throughput** ratio = HEAD / parent (>1 is better).
   * **AverageTime** speedup = parent / HEAD (>1 is better).
   * Conclusions rest on **order-of-magnitude and structural separation**, not 
±1% micro-deltas.
   
   Raw JSON (local run artifacts, not committed):
   
   * `/tmp/groovy-12191-perf-v2/head/scoped-results.json`
   * `/tmp/groovy-12191-perf-v2/parent/scoped-results.json`
   * `/tmp/groovy-12191-perf-v2/head/callsite-results.json`
   * `/tmp/groovy-12191-perf-v2/parent/callsite-results.json`
   * `/tmp/groovy-12191-perf-v2/summary.json`
   
   ---
   
   ## 4. Results
   
   ### 4.1 ScopedInvalidationBench (Throughput, ops/ms — higher is better)
   
   | Benchmark | Parent | HEAD | HEAD/Parent | Interpretation |
   |---|---:|---:|---:|---|
   | `hotLoop_baseline` | 3.931 ± 0.207 | 4.068 ± 0.113 | **1.03×** | H1: no 
hot-path regression |
   | `hotLoop_afterUnrelatedBurst` | 3.889 ± 0.242 | 4.360 ± 0.289 | **1.12×** 
| H3: full speed after unrelated burst |
   | `hotLoop_unrelatedMetaClassChurn` | 0.002278 ± 0.000316 | 0.4486 ± 0.124 | 
**≈197×** | **H2 primary win** |
   | `hotLoop_sameTypeMetaClassChurn` | 0.002327 ± 0.000251 | 0.01892 ± 0.00132 
| **≈8.1×** | Still forced re-link; narrower blast radius than parent |
   | `hotLoop_categoryEnterLeave` | 0.002350 ± 0.000136 | 0.002194 ± 0.000421 | 
**≈0.93×** | H4: bulk semantics retained (parity within noise) |
   | `hierarchy_baseline` | 3.295 ± 0.142 | 2.791 ± 0.117 | 0.85× | Both 
full-speed order; container noise |
   | `hierarchy_parentMetaClassChurn` | 0.002352 ± 0.000169 | 0.01021 ± 
0.000709 | **≈4.3×** | H4: parent change still invalidates child sites |
   
   **Structural fingerprint on HEAD only:**
   
   ```text
   baseline        ≈ 4.07 ops/ms
   after burst     ≈ 4.36 ops/ms   (≥ baseline: no residual deopt)
   unrelated churn ≈ 0.45 ops/ms   (~9× slower than baseline: mostly MetaClass 
write cost)
   same-type churn ≈ 0.019 ops/ms  (~215× slower than baseline: write + 
same-class re-link)
   category        ≈ 0.002 ops/ms  (bulk re-link tax)
   ```
   
   On parent, `unrelated` and `same-type` **both collapse to ~0.002**, showing 
that the old global SwitchPoint made “unrelated churn” equivalent to “global 
deopt.” HEAD separates them by ~**24×** (0.449 / 0.019)—a direct fingerprint 
that scoping is live.
   
   ### 4.2 CallSiteInvalidationBench (AverageTime, ms/op — lower is better)
   
   | Benchmark | Parent | HEAD | Speedup (P/H) | Interpretation |
   |---|---:|---:|---:|---|
   | `baselineHotLoop` | 0.264 ± 0.021 | 0.252 ± 0.015 | **1.05×** | H1 |
   | `baselineListSize` | 0.241 ± 0.004 | 0.250 ± 0.018 | **0.96×** | H1 
(noise) |
   | `baselineSteadyStateNoBurst` | 0.608 ± 0.021 | 0.586 ± 0.020 | **1.04×** | 
H1 |
   | `baselineMultipleCallSites` | 20.82 ± 1.56 | 19.01 ± 1.09 | **1.10×** | H1 
|
   | `crossTypeInvalidationEvery10000` | 306.1 ± 33.1 | 2.503 ± 0.653 | 
**≈122×** | H2 |
   | `crossTypeInvalidationEvery1000` | 428.4 ± 49.0 | 2.250 ± 0.894 | 
**≈190×** | **H2 primary win** |
   | `crossTypeInvalidationEvery100` | 139.2 ± 12.8 | 7.916 ± 0.701 | **≈18×** 
| Still large under denser churn |
   | `listSizeWithCrossTypeInvalidation` | 323.4 ± 56.4 | 1.750 ± 0.394 | 
**≈185×** | JDK-type hot sites benefit too |
   | `multipleCallSitesWithInvalidation` | 1103 ± 56.2 | 25.68 ± 5.97 | 
**≈43×** | Multi-site still much improved |
   | `burstThenSteadyState` | 154.7 ± 23.0 | 1.361 ± 0.112 | **≈114×** | 
“Framework MC extend + request steady state” |
   | `sameTypeInvalidationEvery1000` | 410.6 ± 23.5 | 51.45 ± 2.06 | **≈8.0×** 
| Same-type still slow; local invalidation cheaper than global rotate |
   
   **Penalty vs baseline on HEAD:**
   
   | Scenario | Relative cost | Meaning |
   |---|---|---|
   | crossType @1000 | 2.25 / 0.25 ≈ **9×** `baselineHotLoop` | Dominated by 
~100 ColdType MetaClass writes, not hot-site deopt |
   | sameType @1000 | 51.5 / 0.25 ≈ **205×** `baselineHotLoop` | Clear re-link 
tax |
   | burstThenSteady | 1.36 / 0.59 ≈ **2.3×** steady baseline | Burst write 
cost dominates; steady calls recovered |
   
   On parent, crossType@1000 and sameType@1000 are **both ~410–428 ms/op**, 
again confirming global invalidation collapsed cross-type into full deopt.
   
   ### 4.3 Comparison to the earlier ae8e497-only report
   
   An earlier local report (posted on PR #2736 for `ae8e497` alone) measured 
~146× on `crossType@1000` and ~156× on scoped unrelated thrpt. This re-run on 
**`f48ab19`** measures ~**190×** / ~**197×** on the same primary rows. 
Differences of tens of percent on multi-hundred-× ratios are expected on a 
6-vCPU shared host; the **structural story is unchanged**:
   
   * baselines stay ~1×,
   * cross-type improves by **10¹–10²×**,
   * same-type / category remain correctly expensive,
   * HEAD separates unrelated from same-type by ~20× where parent could not.
   
   `f48ab19` is primarily a **correctness / scalability / review-hygiene** 
commit (index, array lattice, lock restore, docs). It is **not** expected to 
move monomorphic hot-path baselines; the A/B above confirms that.
   
   ---
   
   ## 5. Why it is faster
   
   ```text
                       Old path                              New path
     ColdType.metaClass.foo = ...           ColdType.metaClass.foo = ...
               │                                      │
               ▼                                      ▼
      invalidate global SwitchPoint        invalidate ClassInfo(ColdType) SP
               │                           (+ ClassHierarchyIndex descendants)
               ▼                                      ▼
     ALL sites: guard fails                ColdType (+ subtypes) sites only
     HotTarget.compute → full re-select    HotTarget.compute → stays linked
     (MH rebuild / re-guard / cold path)   (single SwitchPoint check passes)
   ```
   
   Cost breakdown:
   
   1. **Avoided re-links** — After each global invalidation, hot sites re-run 
`Selector`, reinstall guards, and rewrite `CallSite`s. At 100 churn events × 
many sites this dominates (parent’s 400+ ms/op).
   2. **Avoided JIT churn** — Frequent `SwitchPoint.invalidateAll` breaks 
monomorphic inlining assumptions and amplifies deopt/recompile cost (especially 
under dense cross-type churn).
   3. **Unchanged hot-path guard count** — Still one `guardWithTest`, so 
baselines stay flat (H1).
   4. **Same-type / hierarchy still correctly charged** — Prevents “fast but 
wrong” semantics (H4 + 76 tests).
   5. **Category remains bulk** — No second hot-path guard; category visibility 
stays simple and correct; cost matches the old global path (measured ~1×).
   6. **Indexed fan-out (`f48ab19`)** — Typed MetaClass invalidation walks only 
registered descendants, not every loaded class. Correctness for `Object[]` / 
interface arrays / multi-dim arrays is restored without reintroducing a global 
SP.
   
   Same-type still improves ~**8×** on HEAD vs parent, not because re-link is 
skipped, but because the invalidation surface shrinks from “all process sites” 
to “this class hierarchy” and batch `invalidateAll` is cheaper—a secondary 
benefit, not the primary goal.
   
   ---
   
   ## 6. Risks and boundaries
   
   | Topic | Assessment |
   |---|---|
   | Hot-path regression | **Not observed** (baselines 0.96–1.10×) |
   | Semantic regression | 76/76 tests for this tip; Category / hierarchy / 
per-instance MC / array lattice covered |
   | Dense Category enter/leave | **Still expensive** (by design); 
Category-heavy workloads do not get faster |
   | Hierarchy baseline 0.85× | Same full-speed band as parent; treat as noise 
on shared 6-vCPU host, not a design regression |
   | Non-final parent MetaClass change | Fan-out is O(\|indexed subtypes\|) 
after `f48ab19` (was full ClassInfo walk in `ae8e497`) |
   | Category bulk | Still walks all loaded ClassInfos; detach is a cheap no-op 
when no SP was allocated |
   | Legacy `IndyInterface.switchPoint` | Rotated for binary compatibility 
(bulk paths only, under lock again); **no longer** the real call-site MOP guard 
|
   | Environment noise | Shared/container CPUs; conclusions use 
order-of-magnitude gaps (10¹–10²×), robust to ±10% noise |
   | Not covered here | Concurrent multi-thread churn throughput, full Grails 
end-to-end, JDK 17/21 matrix (recommended for CI) |
   
   ---
   
   ## 7. Hypothesis scorecard
   
   | Hypothesis | Result | Evidence |
   |---|---|---|
   | H1 baseline parity | **Holds** | Scoped baseline 1.03×; CallSite baselines 
0.96–1.10× |
   | H2 cross-type speedup | **Holds** | ≈197× thrpt; ≈190× avgt (@1000) |
   | H3 post-unrelated-burst full speed | **Holds** | `afterUnrelatedBurst` ≥ 
baseline on HEAD |
   | H4 necessary invalidation still paid | **Holds** | same-type / hierarchy / 
category ≪ baseline |
   | H5 correctness | **Holds** | 76/76 tests (incl. `ClassHierarchyIndexTest`) 
|
   
   ---
   
   ## 8. Conclusions and recommendations
   
   ### Verdict
   
   `ae8e497` + `f48ab19` 
([GROOVY-12191](https://issues.apache.org/jira/browse/GROOVY-12191), [PR 
#2736](https://github.com/apache/groovy/pull/2736)) keep a **single-guard 
monomorphic hot path** while scoping indy MOP invalidation to per-class domains 
(+ indexed hierarchy / array lattice). For **unrelated-type MetaClass churn 
with hot monomorphic call sites**—a Grails/framework-shaped load—measured 
improvement is on the order of **10¹–10²×** (primary rows ≈ **190–197×** on 
this host). Steady-state undisturbed paths show **no material regression**. 
Correctness costs for Category and same-type changes **remain visible and 
intentional**. Review follow-ups (lock restore, docs, array fan-out, subtype 
index) do not undo the win.
   
   **Performance verification: PASS.**
   
   ### Recommendations
   
   1. Keep `ScopedInvalidationBench` and the cross-type / burst rows of 
`CallSiteInvalidationBench` in regular JMH regression (README already documents 
the suite) so a return to a global SwitchPoint cannot land silently.
   2. In release notes, state clearly that Category and unscoped MetaClass 
events may still bulk-invalidate; the win is **type-local MetaClass change**.
   3. Document migration away from reading `IndyInterface.switchPoint` (field 
is `@Deprecated`; bulk-only rotation).
   4. Optionally re-run CallSite `crossType@1000` and baselines on JDK 17/21 to 
confirm cross-LTS consistency.
   5. CI “Performance Alert” on throughput benches that report higher ops/ms as 
“worse” remains a bot false-positive class; prefer calibrated geomean summaries 
for PR triage.
   
   ---
   
   ## Appendix A — Reproduction commands
   
   ```bash
   # Correctness
   ./gradlew :test --tests Groovy12191 \
     --tests org.apache.groovy.runtime.indy.IndyInvalidationTest \
     --tests org.apache.groovy.runtime.indy.SwitchPointInvalidatorTest \
     --tests org.apache.groovy.runtime.indy.ClassHierarchyIndexTest \
     --tests org.codehaus.groovy.vmplugin.v8.IndyScopedSwitchPointTest
   
   # Performance (HEAD @ f48ab19)
   ./gradlew :perf:jmh -PbenchInclude=ScopedInvalidation -PjmhResultFormat=JSON
   ./gradlew :perf:jmh -PbenchInclude=CallSiteInvalidation 
-PjmhResultFormat=JSON
   
   # Performance (parent @ 0ca665da; same ScopedInvalidationBench + 
CallSiteInvalidationBench sources)
   cd /path/to/parent && ./gradlew :perf:jmh -PbenchInclude=ScopedInvalidation 
-PjmhResultFormat=JSON
   cd /path/to/parent && ./gradlew :perf:jmh 
-PbenchInclude=CallSiteInvalidation -PjmhResultFormat=JSON
   ```
   
   ## Appendix B — Files touched (combined)
   
   **`ae8e497` — scoping core**
   
   * New: `org.apache.groovy.runtime.indy.IndyInvalidation`, 
`SwitchPointInvalidator`, package-info
   * Updated: `ClassInfo`, `IndyInterface`, `Selector`, 
`ColdReflectiveMethodHandleWrapper`, `IndyCompoundAssign`
   * Tests: `Groovy12191`, `IndyInvalidationTest`, 
`SwitchPointInvalidatorTest`, `IndyScopedSwitchPointTest`
   * Benchmarks: `ScopedInvalidationBench`; docs: 
`subprojects/performance/README.adoc`
   
   **`f48ab19` — review follow-ups + index**
   
   * New: `ClassHierarchyIndex`, `ClassHierarchyIndexTest`
   * Updated: `IndyInvalidation` (index-backed fan-out), `ClassInfo` 
(register), `IndyInterface` (sync + deprecation metadata), docs/tests for array 
/ `incVersion` / bulk paths
   
   ---
   
   *Report generated from local JMH A/B measurements on 2026-07-26 (re-run on 
tip `f48ab19` vs baseline `0ca665da`). 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to