[
https://issues.apache.org/jira/browse/GROOVY-12316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109352#comment-18109352
]
ASF GitHub Bot commented on GROOVY-12316:
-----------------------------------------
paulk-asert merged PR #2840:
URL: https://github.com/apache/groovy/pull/2840
> 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)
