[
https://issues.apache.org/jira/browse/GROOVY-12316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109120#comment-18109120
]
ASF GitHub Bot commented on GROOVY-12316:
-----------------------------------------
codecov-commenter commented on PR #2840:
URL: https://github.com/apache/groovy/pull/2840#issuecomment-5450026901
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2840?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 `62.50000%` with `3 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 67.2960%. Comparing base
([`a38552c`](https://app.codecov.io/gh/apache/groovy/commit/a38552c034a9c1c3237ed23fcbe1876a86d6cb80?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`8e3661a`](https://app.codecov.io/gh/apache/groovy/commit/8e3661ab31423b6cb9f89d28b30de16eb6cd1a18?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2840?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[.../parser/antlr4/internal/atnmanager/AtnManager.java](https://app.codecov.io/gh/apache/groovy/pull/2840?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fparser%2Fantlr4%2Finternal%2Fatnmanager%2FAtnManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9wYXJzZXIvYW50bHI0L2ludGVybmFsL2F0bm1hbmFnZXIvQXRuTWFuYWdlci5qYXZh)
| 62.5000% | [0 Missing and 3 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2840?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/2840?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## GROOVY_5_1_X #2840 +/- ##
======================================================
- Coverage 67.3026% 67.2960% -0.0066%
Complexity 29645 29645
======================================================
Files 1384 1384
Lines 117098 117099 +1
Branches 20595 20594 -1
======================================================
- Hits 78810 78803 -7
- Misses 31744 31748 +4
- Partials 6544 6548 +4
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2840?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[.../parser/antlr4/internal/atnmanager/AtnManager.java](https://app.codecov.io/gh/apache/groovy/pull/2840?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fparser%2Fantlr4%2Finternal%2Fatnmanager%2FAtnManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9wYXJzZXIvYW50bHI0L2ludGVybmFsL2F0bm1hbmFnZXIvQXRuTWFuYWdlci5qYXZh)
| `77.5000% <62.5000%> (+5.7051%)` | :arrow_up: |
... and [6 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2840/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>
> groovy.antlr4.cache.threshold silently disables pressure-based DFA clearing
> ---------------------------------------------------------------------------
>
> Key: GROOVY-12316
> URL: https://issues.apache.org/jira/browse/GROOVY-12316
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h2. Problem
> {{AtnManager}} has two mechanisms for dropping the shared parser DFA cache,
> but they are mutually exclusive rather than composable:
> * a *GC canary* — the softly referenced {{AtnWrapper}}. Its collection
> signals memory pressure, so the cache is dropped when the next parse observes
> the cleared reference.
> * a *parse counter* — clears every {{groovy.antlr4.cache.threshold}} parses.
> {{isSmartCleanupEnabled()}} is defined as {{DFA_CACHE_THRESHOLD == 0}}, and
> the canary clear is guarded by {{shouldClearDfaCache() &&
> isSmartCleanupEnabled()}}. Setting *any* positive threshold therefore
> switches the canary off.
> Since the threshold is the documented knob for bounding DFA cache growth, a
> user reaching for it to reduce memory use silently removes the only mechanism
> that responds to actual memory pressure — and can end up worse off than
> before.
> This was hit in practice: a user chasing an OOM on 5.1.1 under Maven at
> {{-Xmx128m}} set {{-Dgroovy.antlr4.cache.threshold=200}} as a mitigation,
> which made *5.1.0* start OOMing too, because it disabled the valve that had
> been keeping it alive.
> h2. Reproduction
> In a forked JVM with {{-Dgroovy.antlr4.cache.threshold=25}}, parse 12 sources
> (fewer than the threshold, so the counter cannot fire), force the JVM to
> clear soft references, then parse once more and count live states in
> {{GroovyParser._ATN.decisionToDFA}}:
> || || DFA states populated || after soft refs cleared ||
> | current | 478 | 486 — not reclaimed |
> | composed | 478 | drops — reclaimed by the canary |
> h2. Fix
> Derive two independent switches from the raw property instead of overloading
> one:
> {code:java}
> GC_CANARY_ENABLED = gcCanaryEnabled(t); // t >= 0
> DFA_CACHE_THRESHOLD = counterThreshold(t); // max(t, 0)
> {code}
> The canary clear is then guarded by {{GC_CANARY_ENABLED}} and the parse
> counter by {{isThresholdCleanupEnabled()}}, so both can be active at once.
> || threshold || before || after ||
> | {{0}} (default) | canary on, counter off | unchanged |
> | {{> 0}} | canary *off*, counter on | canary *on*, counter on |
> | {{< 0}} | never clear | unchanged (never clear) |
> Behaviour changes only in the {{> 0}} case. The negative value remains the
> explicit "never clear" escape hatch — it now switches off both mechanisms,
> since "canary on" is no longer implied by a zero threshold.
> h2. Notes
> * The default configuration is unchanged, so this carries no throughput cost
> for users who do not set the property.
> * This makes the knob usable; it does not on its own close the 5.1.1 memory
> regression. At the default the canary is still only observed on the parse
> path, so under a tight heap the cache can go unreclaimed until an OOM that
> arrives mid-parse. Closing that needs either a non-zero default threshold or
> holding the DFA cache softly in the ANTLR runtime.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
