You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "strongduanmu (via GitHub)" <gi...@apache.org> on 2026/05/20 09:52:19 UTC
[PR] Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 [shardingsphere]
strongduanmu opened a new pull request, #38707:
URL: https://github.com/apache/shardingsphere/pull/38707
Fixes #ISSUSE_ID.
Changes proposed in this pull request:
- Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8
---
Before committing this PR, I'm sure that I have checked the following options:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit tests for my changes.
- [ ] I have updated the Release Notes of the current development version. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 [shardingsphere]
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on PR #38707:
URL: https://github.com/apache/shardingsphere/pull/38707#issuecomment-4497521039
Merge Verdict: Mergeable
Reviewed Scope
- kernel/transaction/core/src/main/java/org/apache/shardingsphere/transaction/savepoint/ConnectionSavepointManager.java
- proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/PostgreSQLPortalContextRegistry.java
- .codex/skills/review-pr/SKILL.md
Not Reviewed Scope
- GitHub check status, per reviewer instruction.
- Local CI/E2E rerun, because the author/reviewer confirmed they already passed locally.
- Performance benchmark, because this is a known JDK 8 ConcurrentHashMap#computeIfAbsent optimization pattern.
Review Basis
The runtime changes directly address the intended root cause: avoiding the JDK 8 ConcurrentHashMap#computeIfAbsent hot-path performance issue when the cached value already exists.
Both modified runtime paths now follow the expected pattern:
1. Read the cache first with get.
2. Call computeIfAbsent only on cache miss.
3. Return the resolved cached value.
This preserves the original lazy initialization behavior while avoiding unnecessary computeIfAbsent calls on cache hits.
For ConnectionSavepointManager, the refactoring extracts the savepoint map lookup into getConnectionSavepoints, keeping the existing behavior of creating a per-connection savepoint map only when absent.
For PostgreSQLPortalContextRegistry, the portal context lookup keeps the same per-connection cache semantics and still creates a new PortalContext only when the connection id has no cached context.
The .codex/skills/review-pr/SKILL.md update is aligned with the PR intent because it codifies the same review rule for future high-frequency Proxy/JDBC paths.
Risk Assessment
- Design: acceptable. The change is localized and keeps the existing cache ownership unchanged.
- Performance: improved for cache-hit paths on JDK 8.
- Compatibility: no public API, SPI, SQL semantics, config, or protocol behavior changes.
- Regression risk: low. Cache miss behavior remains guarded by computeIfAbsent, so concurrent initialization semantics are preserved.
- Documentation: no user-facing documentation update required.
Pre-Merge Checks
- Local CI and E2E were reported as passed.
- No extra performance test is required for this known JDK 8 optimization pattern.
This PR is safe to merge under the reviewed scope.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 [shardingsphere]
Posted by "zhaojinchao95 (via GitHub)" <gi...@apache.org>.
zhaojinchao95 merged PR #38707:
URL: https://github.com/apache/shardingsphere/pull/38707
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add pre get to avoid ConcurrentHashMap#computeIfAbsent performance problem in JDK 8 [shardingsphere]
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on PR #38707:
URL: https://github.com/apache/shardingsphere/pull/38707#issuecomment-4497127793
<img width="1438" height="736" alt="image" src="https://github.com/user-attachments/assets/cc08fb56-8cef-4885-8cf1-197f3f63f27a" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org