You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by zh...@apache.org on 2026/05/20 10:43:52 UTC
(shardingsphere) branch master updated: Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 (#38707)
This is an automated email from the ASF dual-hosted git repository.
zhaojinchao95 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 58958c5e1b1 Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 (#38707)
58958c5e1b1 is described below
commit 58958c5e1b1e677c70c2f5280f3c342703c1e43a
Author: Zhengqiang Duan <du...@apache.org>
AuthorDate: Wed May 20 18:43:45 2026 +0800
Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 (#38707)
---
.codex/skills/review-pr/SKILL.md | 7 ++++++-
.../transaction/savepoint/ConnectionSavepointManager.java | 10 +++++++++-
.../postgresql/command/PostgreSQLPortalContextRegistry.java | 6 +++++-
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/.codex/skills/review-pr/SKILL.md b/.codex/skills/review-pr/SKILL.md
index c4863ea4be9..c28f62c3aa3 100644
--- a/.codex/skills/review-pr/SKILL.md
+++ b/.codex/skills/review-pr/SKILL.md
@@ -67,6 +67,9 @@ description: >-
- If official documentation support is missing, ambiguous, or contradicts the PR behavior, bias to `Merge Verdict: Not Mergeable`.
- If any related dialect remains unresolved, or the review skips the family scan, bias to `Merge Verdict: Not Mergeable`.
- Do not recommend unsupported or undocumented SQL syntax in review feedback.
+16. If a method reachable from the Proxy or JDBC DML/DQL high-frequency SQL execution path uses `ConcurrentHashMap#computeIfAbsent`,
+ require a preceding `get` lookup and call `computeIfAbsent` only when the value is missing, to avoid the JDK 8 implementation bottleneck.
+ If this pattern is absent and the path is high-frequency, bias to `Merge Verdict: Not Mergeable`.
## Execution Boundary
@@ -142,6 +145,7 @@ When information gaps block mergeability, request at least:
4. Risk scan:
- Design: abstraction level, responsibility boundaries, temporary compatibility branches
- Performance: new loops/remote calls/object allocations on hot paths
+ - Performance: in Proxy/JDBC DML/DQL high-frequency SQL paths, flag direct `ConcurrentHashMap#computeIfAbsent` use without a preceding `get` miss check
- Compatibility: behavior/config/API-SPI/SQL dialect versions
- Regression: similar statements, adjacent features, exception branches
- For parser, binder, routing, and default-schema changes, explicitly compare the new behavior against official dialect semantics and check whether precedence or shadowing rules changed
@@ -189,7 +193,7 @@ If the root-cause chain cannot be fully proven fixed, set `Merge Verdict: Not Me
## Risk Checklist (Must Cover)
- Design risk: broken layering, duplicated logic, bypassed SPI/metadata cache, implicit state.
-- Performance risk: complexity increase, extra hot-path allocations, unbounded retries, blocking I/O.
+- Performance risk: complexity increase, extra hot-path allocations, unbounded retries, blocking I/O, or direct `ConcurrentHashMap#computeIfAbsent` in Proxy/JDBC DML/DQL high-frequency SQL paths without a preceding `get` miss check.
- Compatibility risk:
- Behavior compatibility
- Config compatibility
@@ -286,3 +290,4 @@ Use committer tone, gentle wording, no emojis; structure:
- Do not include emojis in change request text.
- Do not output `Mergeable` for a shared-code change unless you have checked at least one non-target dialect or feature that also uses the changed path.
- Do not output `Mergeable` when local verification omitted `-am` on a module-scoped Maven run and dependency freshness matters.
+- Do not output `Mergeable` when Proxy/JDBC DML/DQL high-frequency SQL paths directly call `ConcurrentHashMap#computeIfAbsent` without a preceding `get` miss check.
diff --git a/kernel/transaction/core/src/main/java/org/apache/shardingsphere/transaction/savepoint/ConnectionSavepointManager.java b/kernel/transaction/core/src/main/java/org/apache/shardingsphere/transaction/savepoint/ConnectionSavepointManager.java
index fba6ffca153..b705dc80278 100644
--- a/kernel/transaction/core/src/main/java/org/apache/shardingsphere/transaction/savepoint/ConnectionSavepointManager.java
+++ b/kernel/transaction/core/src/main/java/org/apache/shardingsphere/transaction/savepoint/ConnectionSavepointManager.java
@@ -60,7 +60,15 @@ public final class ConnectionSavepointManager {
*/
public void setSavepoint(final Connection connection, final String savepointName) throws SQLException {
Savepoint result = connection.setSavepoint(savepointName);
- CONNECTION_SAVEPOINT_MAP.computeIfAbsent(connection, unused -> new LinkedHashMap<>()).put(savepointName, result);
+ getConnectionSavepoints(connection).put(savepointName, result);
+ }
+
+ private Map<String, Savepoint> getConnectionSavepoints(final Connection connection) {
+ Map<String, Savepoint> result = CONNECTION_SAVEPOINT_MAP.get(connection);
+ if (null == result) {
+ result = CONNECTION_SAVEPOINT_MAP.computeIfAbsent(connection, unused -> new LinkedHashMap<>());
+ }
+ return result;
}
/**
diff --git a/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/PostgreSQLPortalContextRegistry.java b/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/PostgreSQLPortalContextRegistry.java
index d57ffdd25ca..d4e3bda5caa 100644
--- a/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/PostgreSQLPortalContextRegistry.java
+++ b/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/PostgreSQLPortalContextRegistry.java
@@ -49,7 +49,11 @@ public final class PostgreSQLPortalContextRegistry {
* @return PostgreSQL portal context
*/
public PortalContext get(final int connectionId) {
- return portalContexts.computeIfAbsent(connectionId, unused -> new PortalContext());
+ PortalContext result = portalContexts.get(connectionId);
+ if (null == result) {
+ result = portalContexts.computeIfAbsent(connectionId, unused -> new PortalContext());
+ }
+ return result;
}
/**