[
https://issues.apache.org/jira/browse/GROOVY-12281?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18106129#comment-18106129
]
ASF GitHub Bot commented on GROOVY-12281:
-----------------------------------------
paulk-asert opened a new pull request, #2820:
URL: https://github.com/apache/groovy/pull/2820
### What this PR is
The GROOVY-12281 investigation branch, opened for review of one shippable
change plus its
supporting evidence. **Only `2ab40501e2` ("soft GroovyClassValue mode — soft
values with
resurrection (opt-in)") is intended to land on master** — ideally
cherry-picked or landed
alone rather than squash-merging the branch. The other commits are
investigation artifacts
that should stay on the branch:
- `45e3fc6a56` / `a429dd7415` — the *hybrid* prototype (measured,
**declined**) and the guide
update quantifying the existing `=false` escape hatch (the guide change is
independently
master-worthy if wanted);
- `12d59e847b` — the assessment document (v2) and the loader/perf spike
harnesses:
[GROOVY-12281-assessment.html](https://github.com/apache/groovy/blob/groovy12281/GROOVY-12281-assessment.html).
### Problem
A `java.lang.ClassValue` association lives as long as its key class, so
`ClassInfo.globalClassValue` entries on immortal platform classes (String,
Integer, …) hold a
strong chain to the ClassInfo and through it to Groovy's class loader —
leaking every Groovy
copy a container deploys and undeploys (JDK-8136353 / GROOVY-12142). The
existing remedy,
`-Dgroovy.use.classvalue=false`, gives up the per-Class fast path for every
key
(~4× on the raw lookup, +4.8% geomean on classic dispatch, hot platform
idioms to 1.38×).
### Change
`-Dgroovy.use.classvalue=soft` keeps `ClassValue` for all keys but stores
each value behind a
bootstrap-loaded `SoftReference`, cutting the only strong chain from
immortal keys to the
Groovy island. Three cooperating pieces make that safe (details and file
references in the
assessment):
- **Resurrection** — a weak-key/weak-value side map is the identity
authority: a value still
reachable anywhere is re-associated, never replaced, so a fresh instance
can only exist once
no guard can still observe the old one. Classic call-site version guards
therefore stay
sound: legacy-compiled jars using `groovy-callsite` from the classpath are
unaffected.
- **Dirty roots** — ClassInfos carrying non-reconstructible state (installed
MetaClass,
per-instance MetaClasses, registry-written DGM/extension arrays) are
strong-rooted
Groovy-side; `ClassInfo.remove()` unroots, keeping its hard-detach
undeploy semantics.
- **Per-Class indy domain continuity** — SwitchPoint domains are keyed by
`Class` in soft
mode, so a successor ClassInfo adopts its predecessor's domain and
mutations
deterministically retire guards that captured only the SwitchPoint (POJO
direct dispatch).
Indy-only; the default mode's behaviour is unchanged.
Default-mode behaviour is byte-identical apart from no-op hook calls; all
soft-mode structures
are unallocated unless the flag is set.
### Evidence
- **Acceptance** (the ticket's point; JDK 17/21/23): a dropped child-loader
Groovy copy is
pinned forever under the default and **collected under soft mode** once
pressure clears
soft references.
- **Correctness**: unit tests + a deterministic child-JVM probe
(resurrection identity/version
continuity, DGM rooting, EMC/per-instance survival, recreation dispatch,
classic
`CallSiteArray` soundness, predecessor-SwitchPoint retirement); a real-GC
concurrency
stress probe (128 MB heap, `SoftRefLRUPolicyMSPerMB=0`): 66M dispatches,
~130 EMC
generations, 33/36 platform-receiver ClassInfos collected and recreated
mid-run, zero
invariant violations; full core suite passes under soft mode (16,800
tests).
- **Cost**: raw `getClassInfo` 1.2 → 1.7 ns; compiler harness +0.0%;
classic-bytecode JMH gate
0 of 37 significant (geomean +1.7%; the declined map default failed the
same rule with 12,
worst 1.38×); classic *polymorphic* miss traffic ≈ +3.7% pooled, flagged
for the idiom-suite
sweep before any default-flip discussion.
### Open questions for review
Assessment §9 consolidates the resulting option landscape
(`true`/`soft`/`false` ×
static/dynamic × indy/classic, with guidance); §10 lists the open items:
adoption path
(opt-in now — recommended — vs eventual default), whether domain re-homing
should become
unconditional rather than soft-gated, and container soak experience. The
integration guide is
deliberately untouched until the mode's fate is decided.
JIRA: https://issues.apache.org/jira/browse/GROOVY-12281 (assessment v2
supersedes v1 in place;
this prototype responds to the review comments there)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> Investigate mitigating class-loader pinning by ClassInfo.globalClassValue
> under the default ClassValue implementation
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12281
> URL: https://issues.apache.org/jira/browse/GROOVY-12281
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
> Attachments: GROOVY-12281-Assessment.pdf
>
>
> Follow-up from review discussion on [PR
> #2798|https://github.com/apache/groovy/pull/2798] (GROOVY-12142), preserving
> the analysis from that review. A {{ClassValue}} realizes the chain _key class
> → association → value → everything reachable from it_, and the association
> lives as long as the key class
> ([JDK-8136353|https://bugs.openjdk.org/browse/JDK-8136353], working as
> intended). The {{ClassValue}} implementation class itself prevents no
> unloading; what matters is the key's origin: a Groovy-loaded key class dies
> with Groovy's loader (fine), but a JDK/platform key class is effectively
> immortal, so any value reachable from it that was loaded by Groovy's loader
> pins that loader forever. Indirection counts — a JDK-typed value (e.g. an
> {{ArrayList}}) whose _elements_ are Groovy-loaded re-creates the pin one
> level down.
> {{ClassInfo.globalClassValue}} is static with unknown keys, including
> platform classes ({{String}} receives a {{ClassInfo}} in essentially every
> Groovy program), so the default {{ClassValue}} path pins the loader; today's
> only remedy is the global {{groovy.use.classvalue=false}} escape hatch, which
> trades away the per-class fast path for all keys.
> The general mitigation — {{SoftReference}}-wrapped values with a
> check-remove-recompute protocol — requires that recomputation be legal, and
> for {{ClassInfo}} it is not in general: a {{ClassInfo}} can carry
> non-recomputable state (modified metaclasses, category state), so a
> softly-collected value could silently discard user metaclass customizations.
> Approaches to investigate:
> * soft values while a {{ClassInfo}} is pristine, hardening the reference on
> first mutation — only classes with customized metaclasses would then pin, a
> far smaller set;
> * per-key policy: platform-loader keys get soft/map treatment, Groovy-loader
> keys stay strong (the key-origin rule applied mechanically);
> * splitting {{ClassInfo}} into recomputable and stateful parts;
> * revisiting whether the map-based implementation should become the default,
> with {{ClassValue}} as the opt-in fast path.
> Recompute cost and dispatch-path performance need measurement for any
> candidate.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
