[
https://issues.apache.org/jira/browse/GROOVY-12316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12316:
-------------------------------
Description:
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.
> 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)
