[
https://issues.apache.org/jira/browse/GROOVY-12259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105042#comment-18105042
]
ASF GitHub Bot commented on GROOVY-12259:
-----------------------------------------
blackdrag commented on PR #2786:
URL: https://github.com/apache/groovy/pull/2786#issuecomment-5305336088
Does this replace https://github.com/apache/groovy/pull/2786 ?
> Make category/bulk call-site invalidation O(live SwitchPoint domains) instead
> of O(loaded classes)
> --------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12259
> URL: https://issues.apache.org/jira/browse/GROOVY-12259
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> GROOVY-12258 made process-wide call-site invalidation a no-op in processes
> that never link an indy MOP guard (classic-only bytecode). This follow-up
> addresses the remaining cost for indy and mixed processes: once any indy site
> has linked, every category enter/leave (and custom-MetaClass / unattributed
> registry event) still retires domains by walking every loaded class.
> h3. Problem
> {{IndyInvalidation.retireAllLoadedDomains()}} iterates
> {{ClassInfo.getAllClassInfo()}} — O(all loaded classes), a weak-reference
> pointer chase over tens of thousands of entries in a framework-sized app —
> *twice per {{use}} block* (enter and leave), collecting the comparatively few
> domains that actually hold a live SwitchPoint.
> h3. Proposed fix
> Track live SwitchPoints in a process-wide registry so bulk retirement is
> O(live domains):
> * {{SwitchPointInvalidator}} gains a static {{ConcurrentHashMap<SwitchPoint,
> SwitchPointInvalidator>}} of every live SwitchPoint mapped to its owning
> invalidator. It is keyed by *SwitchPoint*, not invalidator: each SwitchPoint
> has a single-use lifecycle (allocated once, detached once), so a removal can
> never clobber a successor's entry the way an invalidator-keyed registry could
> (ABA on re-allocation).
> * {{getSwitchPoint()}} registers the new SwitchPoint *before* the publishing
> CAS (and deregisters on CAS loss). The GROOVY-12258 ordering argument carries
> over: a bulk path that finds no entry is guaranteed that SwitchPoint was not
> yet visible to any guard, so skipping it is safe.
> * Both detach paths ({{detachLive()}} and the new {{detachIfCurrent(sp)}})
> deregister on successful detach, so all maintenance funnels through the
> existing allocation/retirement choke points; per-class invalidation needs no
> changes.
> * {{retireAllLoadedDomains()}} drains the registry: each entry is claimed via
> {{detachIfCurrent}}, and an entry is removed *only on a successful claim*.
> Removing on a failed claim would strand a concurrently-publishing SwitchPoint
> permanently invisible to all future drains (a correctness trap: pre-publish
> entries must survive the drain); failed-claim entries are transient and are
> cleaned up by their owner.
> * The GROOVY-12258 monotonic flag is replaced by {{registry-is-empty}}, which
> is strictly stronger: it also skips after all domains have retired, and
> re-arms rather than being one-shot. The classic-only skip is preserved.
> The drain's weakly consistent iteration can miss a SwitchPoint published
> mid-drain — the same window as the previous all-classes walk; sites linking
> concurrently read the current category state at link time, unchanged. One
> trade-off to note: the registry holds keys strongly, so a domain whose
> MetaClass has died stays pinned (one small SwitchPoint + invalidator) until
> any bulk event sweeps it.
> h3. Measurements
> JMH ({{-PbenchInclude=CategoryBench.categoryInLoop :perf:jmh}}, 2 forks x 5
> iterations, JDK 21, same machine throughout):
> ||build||indy||classic||
> |Groovy 5 (5.1.x branch)|1489.8 ± 39.1 ms/op|163.2 ± 12.8 ms/op|
> |Groovy 6 pre-GROOVY-12191|1269.1 ± 45.5 ms/op|147.5 ± 12.0 ms/op|
> |master (post-GROOVY-12191)|2141.1 ± 97.8 ms/op|365.9 ± 18.5 ms/op|
> |+ GROOVY-12258|2141.1 ± 97.8 ms/op|59.9 ± 1.2 ms/op|
> |+ this change|1596.1 ± 57.1 ms/op|60.5 ± 1.1 ms/op|
> Indy improves 25% over master; classic keeps the GROOVY-12258 level. On this
> worst-case category-churn bench, indy remains ~26% behind pre-GROOVY-12191:
> the residual is the relink storm (scoped domains retire many per-MetaClass
> SwitchPoints where the old design retired one global one), not the walk.
> Closing that would need a further design change — e.g. a dedicated category
> SwitchPoint axis so category enter/leave stops retiring MetaClass domains —
> and is out of scope here.
> New unit coverage includes a get/detach/drain concurrency hammer asserting no
> live SwitchPoint is ever left unregistered, exactly-once claiming between
> owner detach and bulk drain, and CAS-loser cleanup. All existing
> indy/vmplugin and category runtime tests pass.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
