[
https://issues.apache.org/jira/browse/GROOVY-12259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105154#comment-18105154
]
ASF GitHub Bot commented on GROOVY-12259:
-----------------------------------------
codecov-commenter commented on PR #2786:
URL: https://github.com/apache/groovy/pull/2786#issuecomment-5310608275
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2786?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `94.00000%` with `3 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 70.1353%. Comparing base
([`ad907ac`](https://app.codecov.io/gh/apache/groovy/commit/ad907ac2ac33e6c1382fcf0aa9904170adf20509?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`afd939d`](https://app.codecov.io/gh/apache/groovy/commit/afd939db58c2a933b68e65e125420edb984f3c01?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 15 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2786?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...g/apache/groovy/runtime/indy/IndyInvalidation.java](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Findy%2FIndyInvalidation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2luZHkvSW5keUludmFsaWRhdGlvbi5qYXZh)
| 88.2353% | [1 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...he/groovy/runtime/indy/SwitchPointInvalidator.java](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Findy%2FSwitchPointInvalidator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2luZHkvU3dpdGNoUG9pbnRJbnZhbGlkYXRvci5qYXZh)
| 94.7368% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2786 +/- ##
==================================================
+ Coverage 70.1168% 70.1353% +0.0185%
- Complexity 35772 35799 +27
==================================================
Files 1561 1562 +1
Lines 132362 132387 +25
Branches 24331 24334 +3
==================================================
+ Hits 92808 92850 +42
+ Misses 31156 31139 -17
Partials 8398 8398
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2786?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...java/org/codehaus/groovy/reflection/ClassInfo.java](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Freflection%2FClassInfo.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3JlZmxlY3Rpb24vQ2xhc3NJbmZvLmphdmE=)
| `88.6256% <100.0000%> (+1.0719%)` | :arrow_up: |
|
[...he/groovy/runtime/indy/SwitchPointInvalidator.java](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Findy%2FSwitchPointInvalidator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2luZHkvU3dpdGNoUG9pbnRJbnZhbGlkYXRvci5qYXZh)
| `97.8723% <94.7368%> (-2.1277%)` | :arrow_down: |
|
[...g/apache/groovy/runtime/indy/IndyInvalidation.java](https://app.codecov.io/gh/apache/groovy/pull/2786?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Findy%2FIndyInvalidation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2luZHkvSW5keUludmFsaWRhdGlvbi5qYXZh)
| `87.1287% <88.2353%> (-2.0605%)` | :arrow_down: |
... and [15 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2786/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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)
