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 <[email protected]>
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;
}
/**
