Skip to content

GROOVY-12191: Scope indy SwitchPoint invalidation#2736

Open
daniellansun wants to merge 3 commits into
masterfrom
GROOVY-12191
Open

GROOVY-12191: Scope indy SwitchPoint invalidation#2736
daniellansun wants to merge 3 commits into
masterfrom
GROOVY-12191

Conversation

@daniellansun

Copy link
Copy Markdown
Contributor

@daniellansun daniellansun changed the title Scope indy SwitchPoint invalidation and harden PIC/math paths GROOVY-12191: Scope indy SwitchPoint invalidation and harden PIC/math paths Jul 25, 2026
@asf-gitbox-commits
asf-gitbox-commits force-pushed the GROOVY-12191 branch 4 times, most recently from 5c5a20a to 793abf8 Compare July 25, 2026 13:17

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 1a31f27 Previous: 532d385 Ratio
org.apache.groovy.bench.ClosurePackBench.sum_mono ( {"pack":"false"} ) 3486.862591201315 ops/ms 2192.517293269796 ops/ms 1.59
org.apache.groovy.bench.AryBench.java ( {"n":"1000"} ) 0.11933111295992475 ms/op 0.06377015794088806 ms/op 1.87
org.apache.groovy.bench.CalibrationBench.memoryPointerChase 975.1669314493328 us/op 572.763783987267 us/op 1.70

This comment was automatically generated by workflow using github-action-benchmark.

@codecov-commenter

codecov-commenter commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.60104% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.8966%. Comparing base (c513a3a) to head (1a31f27).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...g/apache/groovy/runtime/indy/IndyInvalidation.java 87.8788% 5 Missing and 3 partials ⚠️
...org/codehaus/groovy/vmplugin/v8/IndyInterface.java 57.8947% 5 Missing and 3 partials ⚠️
...pache/groovy/runtime/indy/ClassHierarchyIndex.java 93.7500% 1 Missing and 2 partials ⚠️
...java/org/codehaus/groovy/reflection/ClassInfo.java 95.8333% 0 Missing and 1 partial ⚠️
...vmplugin/v8/ColdReflectiveMethodHandleWrapper.java 80.0000% 0 Missing and 1 partial ⚠️
...java/org/codehaus/groovy/vmplugin/v8/Selector.java 50.0000% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2736        +/-   ##
==================================================
+ Coverage     69.3657%   69.8966%   +0.5308%     
- Complexity      35082      35209       +127     
==================================================
  Files            1553       1557         +4     
  Lines          131490     130902       -588     
  Branches        24045      23945       -100     
==================================================
+ Hits            91209      91496       +287     
+ Misses          32045      31152       -893     
- Partials         8236       8254        +18     
Files with missing lines Coverage Δ
...he/groovy/runtime/indy/SwitchPointInvalidator.java 100.0000% <100.0000%> (ø)
...odehaus/groovy/vmplugin/v8/IndyCompoundAssign.java 84.0909% <100.0000%> (ø)
...java/org/codehaus/groovy/reflection/ClassInfo.java 92.0792% <95.8333%> (+1.3688%) ⬆️
...vmplugin/v8/ColdReflectiveMethodHandleWrapper.java 92.3077% <80.0000%> (-1.5699%) ⬇️
...java/org/codehaus/groovy/vmplugin/v8/Selector.java 80.4538% <50.0000%> (ø)
...pache/groovy/runtime/indy/ClassHierarchyIndex.java 93.7500% <93.7500%> (ø)
...g/apache/groovy/runtime/indy/IndyInvalidation.java 87.8788% <87.8788%> (ø)
...org/codehaus/groovy/vmplugin/v8/IndyInterface.java 84.0659% <57.8947%> (-3.3626%) ⬇️

... and 115 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

JMH summary — classic (commit 19b09d8)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group Speedup Calibrated n
bench 1.001 × 0.956 × 92
core 1.156 × 0.959 × 77
grails 1.100 × 0.754 × 80

⚠️ Runner speed differs ≥15% from the historical baseline hardware for: core-ag, grails-ad, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.

Runner calibration (this run vs baseline hardware): bench 1.03× (26 rulers) · core-ag 1.45× (3 rulers) · core-hz 0.98× (3 rulers) · grails-ad 1.44× (3 rulers) · grails-ez 1.47× (3 rulers)

Baseline: dev/bench/jmh/<part>/classic/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

JMH summary — indy (commit 19b09d8)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group Speedup Calibrated n
bench 0.978 × 1.010 × 92
core 3.791 × 3.882 × 77
grails 4.341 × 4.526 × 80

Runner calibration (this run vs baseline hardware): bench 0.97× (26 rulers) · core-ag 0.97× (3 rulers) · core-hz 0.98× (3 rulers) · grails-ad 0.99× (3 rulers) · grails-ez 0.94× (3 rulers)

Baseline: dev/bench/jmh/<part>/indy/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data

@asf-gitbox-commits
asf-gitbox-commits force-pushed the GROOVY-12191 branch 5 times, most recently from 8e3d5cf to 9f118c0 Compare July 25, 2026 17:14
@daniellansun daniellansun changed the title GROOVY-12191: Scope indy SwitchPoint invalidation and harden PIC/math paths GROOVY-12191: Scope indy SwitchPoint invalidation Jul 25, 2026
@daniellansun

Copy link
Copy Markdown
Contributor Author

GROOVY-12191 Performance Verification Report

Scoped indy SwitchPoint invalidation

Item Value
Issue GROOVY-12191
Commit under test ae8e49741e74067371823148c8993e8c0b73e317
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; correctness semantics retained

1. Executive summary

This commit changes Groovy’s invokedynamic MOP SwitchPoint invalidation domain from a single process-wide SwitchPoint to a per-class domain attached to ClassInfo (with hierarchy fan-out to loaded subtypes/implementors). 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—the same monomorphic shape as before 6.0.

On the same host, JDK, and JMH annotation settings, A/B measurement shows:

  1. Near-zero regression on steady-state monomorphic hot paths (baseline ratios ≈ 0.99–1.05×, within noise).
  2. Large gains under cross-type MetaClass churn — the primary win:
    • CallSiteInvalidationBench.crossTypeInvalidationEvery1000: 405.6 → 2.77 ms/op (≈ 146×).
    • ScopedInvalidationBench.hotLoop_unrelatedMetaClassChurn: throughput ≈ 156×.
  3. Framework-style “unrelated burst then steady request loop” (burstThenSteadyState): ≈ 76×.
  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.
  5. Correctness: 56/56 tests passed (full scoped-invalidation suite introduced by this commit).

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 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 (parent)

  • 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.

2.2 New model (ae8e497)

Component Role
SwitchPointInvalidator Single-domain lifecycle: CAS get / detachLive / invalidate
IndyInvalidation Policy: invalidateClass (+ hierarchy), invalidateCategory (bulk), guardWithMopSwitchPoints
ClassInfo Holds one indySwitchPoint per class; coordinates version bumps with domain retirement
Selector / IndyCompoundAssign / ColdReflectiveMethodHandleWrapper Install a per-receiver-class SwitchPoint guard at link time

Hot-path shape (intentionally monomorphic):

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 loaded 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

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 afterUnrelatedBurstbaseline
H4 Same-type / hierarchy / category still pay re-link tax Clearly slower than baseline
H5 Correctness suite stays green 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 in container)
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; parent: worktree at 0ca665dad0
Bench source parity ScopedInvalidationBench workload identical on parent (Javadoc-only diff)

3.2 Benchmark suites

  1. org.apache.groovy.bench.ScopedInvalidationBench (added by this commit; 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

./gradlew :test --tests Groovy12191 \
  --tests org.apache.groovy.runtime.indy.IndyInvalidationTest \
  --tests org.apache.groovy.runtime.indy.SwitchPointInvalidatorTest \
  --tests org.codehaus.groovy.vmplugin.v8.IndyScopedSwitchPointTest --rerun-tasks

Result: 56 passed / 0 failed / 0 error.

Test class Cases
bugs.Groovy12191 8
IndyInvalidationTest 27
SwitchPointInvalidatorTest 12
IndyScopedSwitchPointTest 9

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).
  • Builds ran sequentially to avoid dual-JVM core contention; no CPU pinning. Conclusions rest on order-of-magnitude and structural separation, not ±1% micro-deltas.

Raw JSON (local run artifacts, not committed):

  • /tmp/groovy-12191-perf-results/head/scoped-results.json
  • /tmp/groovy-12191-perf-results/parent/scoped-results.json
  • /tmp/groovy-12191-perf-results/head/callsite-results.json
  • /tmp/groovy-12191-perf-results/parent/callsite-results.json

4. Results

4.1 ScopedInvalidationBench (Throughput, ops/ms — higher is better)

Benchmark Parent HEAD HEAD/Parent Interpretation
hotLoop_baseline 3.891 ± 0.224 3.869 ± 0.219 0.99× H1: no hot-path regression
hotLoop_afterUnrelatedBurst 3.774 ± 0.201 4.522 ± 0.168 1.20× H3: full speed after unrelated burst
hotLoop_unrelatedMetaClassChurn 0.00247 ± 0.00017 0.386 ± 0.078 ≈156× H2 primary win
hotLoop_sameTypeMetaClassChurn 0.00249 ± 0.00017 0.0188 ± 0.0012 ≈7.6× Still forced re-link; narrower blast radius than parent
hotLoop_categoryEnterLeave 0.00225 ± 0.00045 0.00231 ± 0.00028 ≈1.03× H4: bulk semantics retained
hierarchy_baseline 3.325 ± 0.118 2.896 ± 0.113 0.87× Noise / load variance; both full-speed order
hierarchy_parentMetaClassChurn 0.00233 ± 0.00021 0.00957 ± 0.00080 ≈4.1× H4: parent change still invalidates child sites

Structural fingerprint on HEAD only:

baseline        ≈ 3.87 ops/ms
unrelated churn ≈ 0.39 ops/ms   (~20× slower than baseline: mostly MetaClass write cost)
same-type churn ≈ 0.019 ops/ms  (~200× 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 ~20× (0.386 / 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.257 ± 0.017 0.248 ± 0.014 1.04× H1
baselineListSize 0.242 ± 0.010 0.236 ± 0.006 1.03× H1
baselineSteadyStateNoBurst 0.612 ± 0.029 0.583 ± 0.033 1.05× H1
baselineMultipleCallSites 20.42 ± 1.03 18.86 ± 0.54 1.08× H1
crossTypeInvalidationEvery10000 288.5 ± 24.3 2.73 ± 1.20 ≈106× H2
crossTypeInvalidationEvery1000 405.6 ± 27.3 2.77 ± 0.88 ≈146× H2 primary win
crossTypeInvalidationEvery100 137.4 ± 10.2 12.5 ± 1.0 ≈11× Still large under denser churn
listSizeWithCrossTypeInvalidation 330.7 ± 25.0 2.21 ± 0.53 ≈150× JDK-type hot sites benefit too
multipleCallSitesWithInvalidation 1098 ± 89 25.9 ± 1.7 ≈42× Multi-site still much improved
burstThenSteadyState 145.8 ± 30.6 1.93 ± 0.16 ≈76× “Framework MC extend + request steady state”
sameTypeInvalidationEvery1000 407.5 ± 29.4 53.3 ± 3.2 ≈7.6× Same-type still slow; local invalidation cheaper than global rotate

Penalty vs baseline on HEAD:

Scenario Relative cost Meaning
crossType @1000 2.77 / 0.25 ≈ 11× baselineHotLoop Dominated by ~100 ColdType MetaClass writes, not hot-site deopt
sameType @1000 53.3 / 0.25 ≈ 215× baselineHotLoop Clear re-link tax
burstThenSteady 1.93 / 0.58 ≈ 3.3× steady baseline Burst write cost dominates; steady calls recovered

On parent, crossType@1000 and sameType@1000 are both ~406 ms/op, again confirming global invalidation collapsed cross-type into full deopt.


5. Why it is faster

                    Old path                              New path
  ColdType.metaClass.foo = ...           ColdType.metaClass.foo = ...
            │                                      │
            ▼                                      ▼
   invalidate global SwitchPoint        invalidate ClassInfo(ColdType) SP
            │                                      │
            ▼                                      ▼
  ALL sites: guard fails                ColdType 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 CallSites. 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 + 56 tests).
  5. Category remains bulk — No second hot-path guard; category visibility stays simple and correct; cost matches the old global path (measured ~1.0×).

Same-type still improves ~7–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.99–1.08×)
Semantic regression 56/56 tests for this commit; Category / hierarchy / per-instance MC covered
Dense Category enter/leave Still expensive (by design); Category-heavy workloads do not get faster
Non-final parent MetaClass change Hierarchy scan over ClassInfo.getAllClassInfo() is O(loaded classes); cold path, correctness-first
Legacy IndyInterface.switchPoint Rotated for binary compatibility only; no longer the real call-site MOP guard; external readers should migrate to IndyInvalidation
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 0.99×; CallSite baselines 1.03–1.08×
H2 cross-type speedup Holds ≈156× thrpt; ≈146× 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 56/56 tests

8. Conclusions and recommendations

Verdict

ae8e497 (GROOVY-12191) keeps a single-guard monomorphic hot path while scoping indy MOP invalidation to per-class domains (+ hierarchy). For unrelated-type MetaClass churn with hot monomorphic call sites—a Grails/framework-shaped load—measured improvement is on the order of 10¹–10²×. Steady-state undisturbed paths show no material regression. Correctness costs for Category and same-type changes remain visible and intentional.

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. Optionally re-run CallSite crossType@1000 and baselines on JDK 17/21 to confirm cross-LTS consistency.
  4. Document migration away from reading IndyInterface.switchPoint (field is @Deprecated).

Appendix A — Reproduction commands

# Correctness
./gradlew :test --tests Groovy12191 \
  --tests org.apache.groovy.runtime.indy.IndyInvalidationTest \
  --tests org.apache.groovy.runtime.indy.SwitchPointInvalidatorTest \
  --tests org.codehaus.groovy.vmplugin.v8.IndyScopedSwitchPointTest

# Performance (HEAD)
./gradlew :perf:jmh -PbenchInclude=ScopedInvalidation -PjmhResultFormat=JSON
./gradlew :perf:jmh -PbenchInclude=CallSiteInvalidation -PjmhResultFormat=JSON

# Performance (parent @ 0ca665dad0; mount the same ScopedInvalidationBench sources for a fair A/B)
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 by the commit

  • New: org.apache.groovy.runtime.indy.IndyInvalidation, SwitchPointInvalidator
  • Updated: ClassInfo, IndyInterface, Selector, ColdReflectiveMethodHandleWrapper, IndyCompoundAssign
  • Tests: Groovy12191, IndyInvalidationTest, SwitchPointInvalidatorTest, IndyScopedSwitchPointTest
  • Benchmarks: ScopedInvalidationBench; docs: subprojects/performance/README.adoc

Report generated from local JMH A/B measurements on 2026-07-26. Raw JSON and run logs: /tmp/groovy-12191-perf-results/.

@paulk-asert

Copy link
Copy Markdown
Contributor

AI read:

PR 2736 (GROOVY-12191) readiness assessment

Bottom line: The design is sound, the timing (early Groovy 6 cycle) is exactly right for a change this deep in the dispatch core, and the evidence base is unusually strong. But it isn't merge-ready today: it's one day old with zero human reviews, the SonarCloud gate is red (all trivially fixable), and there are three or four substantive questions — two of them backward-compatibility related — that deserve an answer on the PR first. I'd call it "approve direction, request changes on details."

What it does

Replaces the single process-wide SwitchPoint guarding all indy MOP call sites with a per-class SwitchPoint domain on ClassInfo (new org.apache.groovy.runtime.indy package: IndyInvalidation policy + SwitchPointInvalidator lifecycle). MetaClass changes for type T now retire only T's SwitchPoint plus loaded subtypes/implementors; category enter/leave and VMPlugin.invalidateCallSites() bulk-retire all class domains. Hot call sites still carry exactly one guard, so steady-state shape is unchanged.

Evidence quality — strong

  • All CI test jobs green across JDK 17/21/25, three OSes, plus dist builds.
  • CI JMH (calibrated): indy core 3.3×, grails suite 4.5×, bench suite parity; classic runtime parity (as expected — it's untouched).
  • The "Performance Alert" on the PR is a false alarm: dispatch_8_megamorphic_java went from 1841 to 2857 ops/ms — that's 1.55× faster; the alert bot applied its smaller-is-better ratio to a throughput metric. Worth a dismissive comment on the PR so it doesn't spook anyone.
  • Daniel's A/B verification report: ~146× on cross-type invalidation churn, ~76× on burst-then-steady, steady-state parity, 56/56 tests. Same-type churn and categories deliberately still pay full re-link cost (correctness preserved, not "falsely optimized").
  • Test coverage is unusually thorough: hierarchy fan-out, interfaces, primitives, cleared ClassInfo weak refs, concurrent detach/get races, cold tier, child-process probes for the frozen log flags.

Backward compatibility — the detailed look you asked for

Binary compatibility: clean. IndyInterface.switchPoint is retained (adding volatile and @Deprecated doesn't affect linkage), all ClassInfo additions are additive and @Internal-annotated, and the build's compat checks passed. Non-indy/legacy callsite caching is unaffected because ClassInfo.getVersion() bump semantics are preserved on every path.

Behavioral compatibility: three real changes, all release-notes material:

  1. IndyInterface.switchPoint no longer means what it meant. It's no longer the guard on any call site, and — the sharper edge — it is no longer rotated on per-class MetaClass changes, only on category enter/leave and invalidateCallSites(). Any external code (it's protected, so subclasses of IndyInterface or anything generating code against it) that installed guards on this field will now silently miss MetaClass-change invalidations and dispatch stale. Exposure is probably tiny, but this must go in the Groovy 6 upgrade notes, and the javadoc's claim that "external readers still see an invalidation" is only true for the category subset of events.

  2. The legacy field rotation lost its lock. The old code rotated switchPoint under synchronized (IndyInterface.class); the new code does an unsynchronized read-replace-invalidate on the volatile. Two concurrent invalidateSwitchPoints() calls can orphan a live SwitchPoint that an external reader just grabbed — that reader then never observes any future invalidation. Since the field is being kept solely for external observers, I'd restore the synchronized block; it's a cold path and the fix is two lines. (This is also effectively what Sonar's S3077 finding is circling.)

  3. ClassInfo.incVersion() (public API) no longer triggers a global flush. It used to call VMPluginFactory.getPlugin().invalidateCallSites(); now it scopes to class+subtypes. External MOP tooling that called incVersion() as a "flush everything" hammer gets weaker semantics. Legitimate design, but it's a documented-behavior change on a public method — upgrade notes again.

One genuine correctness hole to raise: array classes. cannotHaveLoadedSubtypes() treats final classes as fan-out-free, and Class.getModifiers() reports arrays as final — but Object[] is assignable-from every object array type and is the MOP superclass of array metaclasses. So registry.setMetaClass(Object[], emc) (obscure, but legal) retires only Object[]'s domain and leaves a String[]-linked site stale, where the old global SwitchPoint caught it. Needs either a special case (type.isArray() → don't short-circuit) or a test proving it can't matter.

Scalability trade-off worth acknowledging explicitly: invalidation cost went from O(1) (rotate one SwitchPoint) to O(loaded ClassInfos) — every MetaClass change on a non-final type scans getAllClassInfo() with isAssignableFrom, and every category enter/leave detaches every loaded class domain. Batching via invalidateAll keeps it to one safepoint, and the grails suite shows the trade wins decisively in realistic workloads, but Grails-style startup (thousands of EMC mutations × tens of thousands of loaded classes) is now O(N×M) where it was O(N). Fine to accept for alpha; I'd note a follow-up idea (a subtype index on ClassInfo) in the JIRA rather than block on it.

Also flagged for reviewer attention: the invokeColdReflective rewrite is a behavior change beyond pure scoping — it now re-selects via fallback(..., false) and caches the full wrapper into the callsite to prevent cold-tier recursion. Reasonable, but it subtly changes cold-tier promotion and deserves its own review eyes (it's also where most of the uncovered lines in the codecov report live).

Housekeeping before merge

  • SonarCloud gate (red): four findings, all ~10 minutes of work — empty constructor needs removal or comment (S1186), @Deprecated needs since/forRemoval (S6355), S3077 on the volatile field (best answered by restoring the synchronized rotation), S1133 informational.
  • JIRA has no Fix Version — should get 6.0.0-beta-1.
  • No human review yet — for a change that rewires ClassInfo and the indy guard chain, I'd want at least one other MOP-deep committer (Eric or Jochen) to sign off, and given it changes the documented semantics of IndyInterface.switchPoint, a short heads-up on dev@ would be in keeping with how past indy-internals changes were handled.

Groovy 6 fit

Right change, right release, right point in the cycle: it's exactly the kind of behavioral rework that must land in a major, and landing it pre-alpha maximizes soak time across the ecosystem (Grails, Spock, Geb all lean on MetaClass churn patterns this directly targets). Nothing here threatens the Groovy 6 schedule — the open items are days of work, not weeks.

@daniellansun

Copy link
Copy Markdown
Contributor Author

@paulk-asert

Thanks for your review.

Point Action
Array fan-out hole (Object[] final short-circuit) Fixed via ClassHierarchyIndex (full JLS array lattice) + single invalidate path; tests for Object[] / interface arrays / multi-dim
Legacy switchPoint lost lock Restored synchronized (IndyInterface.class)
switchPoint semantics / javadoc over-claim Docs: bulk-only rotation; misses per-class MC; @Deprecated(since="6.0.0", forRemoval=false)
incVersion() no longer global flush Documented + test; bulk callers → invalidateCallSites() / invalidateCategory()
Scalability O(N×M) Shipped weak ancestor→descendant index; typed MC invalidation is O(subtypes). Category still full walk (cheap no-op detach)
Sonar S1186 / S6355 / S3077 Empty-ctor comment; deprecate metadata; sync rotation
invokeColdReflective Documented as intentional cold-tier change (not pure scoping)
Perf Alert bot False alarm: 1841→2857 ops/ms is faster (throughput misread as latency)

Happy to adjust if anything still looks off.

@daniellansun

Copy link
Copy Markdown
Contributor Author

GROOVY-12191 Performance Verification Report V2

Scoped indy SwitchPoint invalidation (re-verification after review follow-ups)

Item Value
Issue 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.
// 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):

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 afterUnrelatedBurstbaseline
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

./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:

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 ≈ 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

                    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 CallSites. 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 ~ 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, PR #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

# 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).

@blackdrag blackdrag left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one point that escapes me right now. Why do we need the hierarchy at all? I don´t think this is for MetaClassImpl cases.

*/
protected static SwitchPoint switchPoint = new SwitchPoint();
@Deprecated(since = "6.0.0", forRemoval = false)
protected static volatile SwitchPoint switchPoint = new SwitchPoint();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all under vmplugin is supposed to be Internal, thus removing instead of deprecating is imho very much allowed. Also why do you make it volatile if you deprecate it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — thank you.

org.codehaus.groovy.vmplugin.* is internal-by-intent. Keeping a dead process-wide SwitchPoint only for binary compatibility was weaker than a clean removal: after scoping, the field was no longer the MOP guard and was rotated only on bulk paths, which was easy to misread.

Action taken

  • Removed protected static SwitchPoint switchPoint entirely.
  • invalidateSwitchPoints() now only bulk-retires per-class domains via IndyInvalidation.invalidateCategory() (no legacy field rotation, no volatile, no extra lock).
  • Docs/package-info updated; tests no longer assert on the removed field.

The earlier volatile was only there to make concurrent bulk rotation of that legacy field observable; with the field gone, that concern disappears.

*/
static MethodHandle applyMopSwitchPoints(final MethodHandle handle, final MethodHandle fallback, final Object receiver) {
return IndyInvalidation.guardWithMopSwitchPoints(handle, fallback, receiver);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the rationale for having the guard creation in IndyInvalidation instead of here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question — the split is intentional, and the previous one-line delegate made it look accidental.

Rationale

Concern Owner
Domain resolution (receiver → class → ClassInfo SwitchPoint) IndyInvalidation (also used by ClassInfo, cold tier, tests)
Invalidation policy (class / hierarchy / category / unscoped) IndyInvalidation
Link-time guardWithTest install on production call sites IndyInterface.applyMopSwitchPoints (Selector, IndyCompoundAssign)

Putting invalidation policy next to bootstrap wiring would grow IndyInterface further and pull ClassInfo toward vmplugin. Putting only the bind step in IndyInterface keeps the bytecode surface as the place that wires handles, while the runtime.indy package owns domains and retire rules.

Action taken

  • applyMopSwitchPoints now performs the bind itself:

    IndyInvalidation.classSwitchPointFor(receiver).guardWithTest(handle, fallback)

  • IndyInvalidation.guardWithMopSwitchPoints remains the public / test entry (same semantics), documented as such.

  • Javadoc on both sides spells out the division of responsibility.

MethodHandleWrapper full = fallback(cold.callSite, cold.sender, cold.methodName, cold.callID,
cold.safeNavigation, cold.thisCall, cold.spreadCall, 1, arguments, false);
cold.callSite.put(receiverCacheKey(arguments[0]),
full.isCanSetTarget() ? full : NULL_METHOD_HANDLE_WRAPPER);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is NULL_METHOD_HANDLE_WRAPPER the fallback for if cannot set target? It may be because of the code path to here, but frankly I think this needs at least a comment as of why this is the only valid version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that this needed an explicit comment — the policy is shared with fromCacheHandle / selectMethodHandle, not cold-tier-specific invention.

Why the sentinel is the only safe PIC value when !canSetTarget

  1. canSetTarget == false means the selection is not class-keyed-cacheable (typical causes: per-instance MetaClass, spread-call → selector.cache == false).
  2. Storing that real wrapper under the receiver-class PIC key would pin a selection that is only valid for one instance or one spread shape, and the next hit would skip re-selection.
  3. NULL_METHOD_HANDLE_WRAPPER is the existing PIC sentinel meaning “do not cache this receiver shape; re-run fallback on the next hit” (see fromCacheHandle).
  4. The current invocation still uses full.getCachedMethodHandle() — the sentinel only shapes the PIC, not the return of this call.

Action taken

  • Expanded the inline comment at the cold-path PIC write to state the above.
  • Added a unit test that asserts the uncacheable → sentinel PIC write policy.

…tion

Remove the legacy process-wide IndyInterface.switchPoint (vmplugin is
internal-by-intent), bind MOP guards at link time in applyMopSwitchPoints,
document hierarchy fan-out for cross-class MOP visibility, and clarify the
cold-path PIC sentinel policy. Extend unit coverage for these paths.
@daniellansun

Copy link
Copy Markdown
Contributor Author

There is one point that escapes me right now. Why do we need the hierarchy at all? I don´t think this is for MetaClassImpl cases.

You are right that this is not because MetaClassImpl shares one method table up the hierarchy. Each class still has its own MetaClass / ClassInfo domain.

Hierarchy fan-out exists for cross-class MOP visibility that already-linked subtype sites must re-observe:

  1. Parent ExpandoMetaClass / registry mutation
    Parent.metaClass.foo = { … } must force sites linked for Child to re-select so new Child().foo() sees the method. Without retiring Child’s SwitchPoint, a previously linked miss (or older target) stays installed.

  2. Interface MetaClass changes
    Same rule for implementors.

  3. Array covariance
    Object[] / interface arrays are MOP-relevant supertypes of reference arrays even though array classes are final — a pure “exact class only” domain would leave String[] sites stale after Object[] MetaClass changes.

So: fan-out is about inherited / covariant MOP state, not about rewriting MetaClassImpl internals. Category enter/leave remains a separate bulk path and does not rely on this index for semantics.

Action taken

  • Documented this rationale on IndyInvalidation, ClassHierarchyIndex, and package-info.
  • Kept / extended tests:
    • parent EMC → child SwitchPoint + runtime visibility
    • parent MetaClassImpl replacement still fans out (registry change can still affect subtype dispatch)
    • unrelated-type churn does not touch an unrelated hierarchy
    • array / interface-array lattice cases (from earlier review)

Happy to discuss a narrower fan-out (e.g. EMC-only) as a follow-up optimization; the current rule prefers correctness and matches pre-6.0 “something in the MOP above me changed → re-link” behaviour, but scoped to the subtype cone instead of the whole process.

@daniellansun
daniellansun requested a review from blackdrag July 27, 2026 13:08
@sonarqubecloud

Copy link
Copy Markdown

@testlens-app

testlens-app Bot commented Jul 27, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 1a31f27
▶️ Tests: 106936 executed
⚪️ Checks: 31/31 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants