Groovy12281 investigation - #2821
Open
paulk-asert wants to merge 6 commits into
Open
Conversation
…ed, declined) Routes platform-loader keys to the weak-key map and other keys to ClassValue, selected via -Dgroovy.use.classvalue=hybrid. Measured against pure ClassValue and pure map with a fresh-JVM-per-config harness: the hybrid tracks the map, not ClassValue, on macro dispatch, because dynamic code cannot avoid platform receivers (String, boxed numbers, DGM) and those are exactly the keys the hybrid maps. Kept on the investigation branch as the measured artifact; not proposed for merge. Numbers and method are recorded in the GROOVY-12281 assessment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… guide The escape hatch's trade-off was described as not measurable for most applications. Measured (57 classic-dispatch JMH benchmarks): +4.8% geomean with hot platform-receiver idioms up to ~1.4x, while compile time is within 1%. The guide now states those numbers so integrators can decide with their own workload in mind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion (opt-in) Adds -Dgroovy.use.classvalue=soft: java.lang.ClassValue keeps the per-Class fast path for all keys, but each association stores a bootstrap-loaded SoftReference to the ClassInfo, cutting the only strong chain from immortal platform classes to Groovy's loader (JDK-8136353 / GROOVY-12142). Correctness pieces, addressing the review findings on the v1 assessment: - Resurrection: a weak-key/weak-value side map is the identity authority; computeValue re-associates a still-reachable value instead of replacing it, so a fresh instance can only exist once no guard can observe the old one (kills the split-brain sequence; classic call-site version guards stay sound, so legacy groovy-callsite jars on the classpath are unaffected). - Dirty roots: ClassInfos carrying non-reconstructible state (installed MetaClass, per-instance MetaClasses, registry-written MOP arrays) are strong-rooted Groovy-side; the MOP-write hook also enforces the registry-rooting invariant (non-empty DGM arrays => never collectible) by construction. ClassInfo.remove() unroots, keeping hard-detach undeploy semantics. - Per-Class indy domain continuity: in soft mode SwitchPoint domains are adopted from a weak-Class-keyed map, so a successor ClassInfo shares its predecessor's domain and mutations deterministically retire guards that captured only the SwitchPoint (POJO direct dispatch); domain reclaim moves from ClassInfo-death to Class-death. Indy-only; default mode unchanged. Tests: GroovyClassValueSoftTest (unit); ClassInfoSoftModeTest/Probe (child JVM, deterministic forced clearing: resurrection identity+version, DGM rooting, EMC and per-instance survival, true collection + fresh dispatch, classic CallSiteArray soundness across clear + EMC change, predecessor SwitchPoint retirement); ClassInfoSoftModeStressTest/Probe (child JVM, real GC clearing: 128MB heap + SoftRefLRUPolicyMSPerMB=0, concurrent indy + classic dispatch racing the collector through ~130 EMC generations — 66M dispatches, 33/36 platform-receiver ClassInfos collected and recreated mid-run, zero invariant violations). build-logic forwards -Pgroovy.use.classvalue to test JVMs so whole suites can soak a non-default mode; the full core suite passes under soft mode (16,800 tests).
…ed, measured Supersedes v1 in place after review: withdraws the blanket "structurally illegal" verdict (Failure 1 cannot occur for collectible ClassInfos — the registry-rooting invariant; Failure 2 is real, demonstrated via the $callSiteArray per-referent-clearing sequence, and prevented by construction via resurrection). Records the reference-graph census (including POGO self-pinning), the indy capture audit, the Option E design, the consolidated option landscape for the adoption discussion (three modes x static/dynamic x indy/classic, with guidance), and the measurements: - Acceptance (Groovy12281LoaderSpike, JDK 17/21/23): dropped child-loader Groovy is pinned forever under the default ClassValue, collected under soft mode once pressure clears soft references — the ticket's first demonstrated unpinning that keeps the ClassValue fast path. - Cost (Groovy12281PerfSpike + compiler harness + classic JMH sweep): +0.6ns on a raw getClassInfo lookup, macro dispatch and compilation at parity, 0 of 37 JMH classic benchmarks significant (geomean +1.7%; Option D failed the same rule with 12); focused re-measurement puts classic polymorphic miss traffic at a pooled +3.7%, flagged for the idiom-suite sweep before any default-flip discussion. v1's measured declines of Options B (hybrid) and D (map default) stand. Recommendation: opt-in mode now; default question deferred to team review.
association; expose value reclaimability as a store capability Jochen's reverse memory-pressure question on PR apache#2820 found a real leak: the global strong root set extended EMC-dirty script-created classes (and their loaders) to the runtime's lifetime, where the default mode releases them with their class (probe: 0/20 loaders collected vs 20/20). The root set's justifying comment was true for immortal platform keys but false for collectible ones. - GroovyClassValue gains valuesReclaimable() plus default-no-op pin/unpin: callers never consult the mode; they ask the store for its capabilities. - GroovyClassValueSoft stores a per-Class slot (bootstrap AtomicReference) holding either a bootstrap SoftReference (reclaimable) or the value itself (pinned). The strong hold lives inside the association — an ephemeron — so a pinned ClassInfo lives exactly as long as its class: an immortal platform key retains it, a dropped script class releases it together with its loader. pin() retries across a concurrent remove-and-recompute; unpin() is a CAS so a foreign value cannot downgrade the slot. - ClassInfo drops nonReclaimableRoots and the isSoftMode() coupling; updateReclaimability() pins/unpins unconditionally (no-ops outside soft mode) and remove() needs no store-specific unroot: the pin travels with the association. - GroovyClassValueFactory stores the parsed mode as booleans; the mode string never leaves the factory. - IndyInvalidation / SwitchPointInvalidator document their layering (mechanism vs policy, with ClassInfo the only other supported consumer) and are annotated @internal. Tests: GroovyClassValueSoftTest adds the capability, pin-until-unpin, remove-releases-pin and foreign-unpin-noop cases; ClassInfoSoftModeProbe gains the reverse scenario (an EMC-dirty script class dies with its loader under memory pressure) and asserts pinned slots directly — the stronger claim, since a pinned slot has no clearable reference. The reverse scenario is @CompileStatic and nulls its loop locals on purpose: indy call-site guards in the long-lived probe class and stale frame slots retain script classes in every mode — receiver-side effects distinct from the association lifetime under test. The classic call-site scenario moves to a clean POJO receiver, since a pinned receiver like String no longer has a clearable reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds §12 (the reverse-scenario leak, its diagnosis and the ephemeron fix; responses to the six inline comments) and updates §2/§6/§7/§8/§9/§10 for the pinned-slot design. Also records the pre-existing reverse leak of groovy.use.classvalue=false on current master (a weak-key/strong-value map is not an ephemeron), to be filed separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚨 TestLens detected 1 failed test 🚨Here is what you can do:
Test SummaryBuild and test / lts (25, ubuntu-latest) > :test
🏷️ Commit: 4d28d2a Test FailuresClassInfoSoftModeStressTest > softModeSurvivesRealGcClearingUnderConcurrency() (:test in Build and test / lts (25, ubuntu-latest))
Rerun ControlsSelect tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app/docs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
not for commit to master - just in case anyone wants to repeat the investigation