[
https://issues.apache.org/jira/browse/CALCITE-7632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Liu Zhengri updated CALCITE-7632:
---------------------------------
Description:
Replace `java.util.Stack` with `java.util.ArrayDeque` in two locations,
completing a TODO left in place since 2020.
h3. Background
In [CALCITE-4314|https://issues.apache.org/jira/browse/CALCITE-4314] (commit
[e537246|https://github.com/apache/calcite/commit/e53724630af60566784e8aa7d8f56fe2932b12cc]),
Vladimir Sitnikov enabled Error Prone checking for the Calcite codebase. He
identified 2 remaining {{java.util.Stack}} usages as obsolete and added
{{@SuppressWarnings("JdkObsolete")}} with {{// TODO: replace with Deque}} to
suppress warnings. The commit message explicitly stated _"It should be replaced
with Deque"_, but the actual refactoring was deferred.
CALCITE-4314 was a broad static-analysis enablement issue — the Stack→Deque
replacement was too granular to be tracked there specifically. This issue
(CALCITE-7632) is a dedicated ticket to complete that deferred work.
h3. Changes
# {{core/src/main/java/org/apache/calcite/runtime/Pattern.java}}:
{{Stack<Pattern>}} → {{Deque<Pattern>}} in {{PatternBuilder.stack}} field (used
for SQL {{MATCH_RECOGNIZE}} NFA pattern matching)
#
{{core/src/main/java/org/apache/calcite/plan/volcano/TopDownRuleDriver.java}}:
{{Stack<Task>}} → {{Deque<Task>}} in task queue (used for Volcano optimizer
top-down Cascades-style optimization)
Also removed the corresponding {{@SuppressWarnings("JdkObsolete")}} annotations
and {{// TODO: replace with Deque}} comments.
h3. Rationale
* {{java.util.Stack}} extends {{Vector}} — all methods are {{synchronized}},
adding unnecessary lock overhead in single-threaded contexts
* {{ArrayDeque}} is non-synchronized, officially recommended by the Java
documentation as a replacement for {{Stack}}
* Eliminates Error Prone {{JdkObsolete}} warnings without suppression
* Better memory expansion: {{ArrayDeque}} doubles capacity on resize vs
{{Vector}}'s linear growth
h3. Related
* PR: [#5062|https://github.com/apache/calcite/pull/5062]
* Origin: [CALCITE-4314|https://issues.apache.org/jira/browse/CALCITE-4314] /
commit
[e537246|https://github.com/apache/calcite/commit/e53724630af60566784e8aa7d8f56fe2932b12cc]
was:
Replace `java.util.Stack` (which extends `Vector` with unnecessary
synchronization overhead) with `java.util.ArrayDeque` in two locations:
1. `Pattern.java` - `PatternBuilder.stack` field (used for SQL MATCH_RECOGNIZE
NFA pattern matching)
2. `TopDownRuleDriver.java` - task queue (used for Volcano optimizer top-down
Cascades-style optimization)
This eliminates the `@SuppressWarnings("JdkObsolete")` annotations and `//
TODO: replace with Deque` comments that have existed since CALCITE-4314 (2020).
Benefits:
- `Stack` extends `Vector` with synchronized methods (unnecessary lock overhead
in single-threaded contexts)
- `ArrayDeque` is non-synchronized and officially recommended by Java
documentation as a replacement for `Stack`
- Removes Error Prone `JdkObsolete` warnings
- Better memory expansion strategy (ArrayDeque doubles capacity vs Vector
linear growth)
> Replace java.util.Stack with java.util.ArrayDeque in Pattern and
> TopDownRuleDriver
> ----------------------------------------------------------------------------------
>
> Key: CALCITE-7632
> URL: https://issues.apache.org/jira/browse/CALCITE-7632
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Liu Zhengri
> Priority: Major
> Labels: pull-request-available
>
> Replace `java.util.Stack` with `java.util.ArrayDeque` in two locations,
> completing a TODO left in place since 2020.
> h3. Background
> In [CALCITE-4314|https://issues.apache.org/jira/browse/CALCITE-4314] (commit
> [e537246|https://github.com/apache/calcite/commit/e53724630af60566784e8aa7d8f56fe2932b12cc]),
> Vladimir Sitnikov enabled Error Prone checking for the Calcite codebase. He
> identified 2 remaining {{java.util.Stack}} usages as obsolete and added
> {{@SuppressWarnings("JdkObsolete")}} with {{// TODO: replace with Deque}} to
> suppress warnings. The commit message explicitly stated _"It should be
> replaced with Deque"_, but the actual refactoring was deferred.
> CALCITE-4314 was a broad static-analysis enablement issue — the Stack→Deque
> replacement was too granular to be tracked there specifically. This issue
> (CALCITE-7632) is a dedicated ticket to complete that deferred work.
> h3. Changes
> # {{core/src/main/java/org/apache/calcite/runtime/Pattern.java}}:
> {{Stack<Pattern>}} → {{Deque<Pattern>}} in {{PatternBuilder.stack}} field
> (used for SQL {{MATCH_RECOGNIZE}} NFA pattern matching)
> #
> {{core/src/main/java/org/apache/calcite/plan/volcano/TopDownRuleDriver.java}}:
> {{Stack<Task>}} → {{Deque<Task>}} in task queue (used for Volcano optimizer
> top-down Cascades-style optimization)
> Also removed the corresponding {{@SuppressWarnings("JdkObsolete")}}
> annotations and {{// TODO: replace with Deque}} comments.
> h3. Rationale
> * {{java.util.Stack}} extends {{Vector}} — all methods are {{synchronized}},
> adding unnecessary lock overhead in single-threaded contexts
> * {{ArrayDeque}} is non-synchronized, officially recommended by the Java
> documentation as a replacement for {{Stack}}
> * Eliminates Error Prone {{JdkObsolete}} warnings without suppression
> * Better memory expansion: {{ArrayDeque}} doubles capacity on resize vs
> {{Vector}}'s linear growth
> h3. Related
> * PR: [#5062|https://github.com/apache/calcite/pull/5062]
> * Origin: [CALCITE-4314|https://issues.apache.org/jira/browse/CALCITE-4314] /
> commit
> [e537246|https://github.com/apache/calcite/commit/e53724630af60566784e8aa7d8f56fe2932b12cc]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
