Optimize DefaultThreadContextMap.getCopy() performance #3939
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vy
added
api
vy
requested changes
Sep 26, 2025
There was a problem hiding this comment.
@jengebr, @anuragdy, thanks so much for the report and the fix. I see that you provided a benchmark in #3935. Curious: Have you checked the existing log4j-perf-test benchmarks on thread context map? Are you able to reproduce the improvement figures using one of the benchmarks there? If so, which one(s)? If not, would you mind updating existing MDC benchmarks accordingly, please?
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
Show resolved
Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
Outdated
Show resolved
Hide resolved
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java
Outdated
Show resolved
Hide resolved
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java
Outdated
Show resolved
Hide resolved
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java
Outdated
Show resolved
Hide resolved
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java
Outdated
Show resolved
Hide resolved
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextMapTest.java
Outdated
Show resolved
Hide resolved
vy
reviewed
Oct 10, 2025
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
Outdated
Show resolved
Hide resolved
vy
approved these changes
Oct 10, 2025
Generated by gradle/develocity-actions |
|
@anuragdy, build is failing, would you mind fixing it, please? See the Log4j Development Guide for details. |
auto-merge was automatically disabled
October 10, 2025 09:07
Head branch was pushed to by a user without write access
anuragdy
added a commit
to anuragdy/logging-log4j2
that referenced
this pull request
Oct 10, 2025
- Fix incorrect indentation in the for loop that was causing compilation failures - Remove redundant performance figures comment as intended in the original commit - Fix duplicate JavaDoc comment that was causing subsequent compilation failures This resolves the GitHub Actions CI build failures in PR apache#3939.
|
@anuragdy, please sign your commits. |
anuragdy
added a commit
to anuragdy/logging-log4j2
that referenced
this pull request
Oct 10, 2025
- Fix incorrect indentation in the for loop that was causing compilation failures - Remove redundant performance figures comment as intended in the original commit - Fix duplicate JavaDoc comment that was causing subsequent compilation failures This resolves the GitHub Actions CI build failures in PR apache#3939.
anuragdy
force-pushed
the
2.x
branch
3 times, most recently
from
c81a658 to
4b981eb
Compare
github-project-automation
bot
moved this from Done
to Waiting for user
in Log4j bug tracker
Thanks for the guidelines, updated. The build workflow is succeeding on the fork now.
Updated the commits. |
vy
reviewed
Oct 10, 2025
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
Outdated
Show resolved
Hide resolved
vy
approved these changes
Oct 10, 2025
github-project-automation
bot
moved this from Waiting for user
to Done
in Log4j bug tracker
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR optimizes the
DefaultThreadContextMap.getCopy()method to address performance issues related to megamorphic calls in the HashMap constructor, providing 30-50% performance improvements for non-empty maps.The original implementation used
new HashMap<>(map)which suffers from megamorphic call performance issues documented in:The HashMap constructor with a Map parameter requires (3 + 4n) virtual method calls that become megamorphic when used with different map types, leading to 24-136% performance degradation compared to manual iteration.
Closes #3935.