-
Notifications
You must be signed in to change notification settings - Fork 20
Trace threadlocal entry is never removed, only nulled #849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This prevents threadlocal map entry churn when traces are added and cleared, however it comes at the potential cost of larger threadlocal maps when tracing is not used. We generally expect everything to be traced, so this shouldn't impact us.
Generate changelog in
|
Hmm, benchmark data isn't helpful because it's using an MDC map implementation which allocates. Using Before
After
|
currentTrace.set(null); | ||
MDC.remove(Tracers.TRACE_ID_KEY); | ||
MDC.remove(Tracers.TRACE_SAMPLED_KEY); | ||
MDC.remove(Tracers.REQUEST_ID_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copying comment from #850
I didn't touch these as I didn't see these pop in profiles, and log4j2 uses
org.apache.logging.log4j.spi.GarbageFreeSortedArrayThreadContextMap
&org.apache.logging.log4j.util.SortedArrayStringMap
where theMDC.remove
&MDC.put
don't directly free/allocate
Released 6.8.0 |
This prevents threadlocal map entry churn when traces are added
and cleared, however it comes at the potential cost of larger
threadlocal maps when tracing is not used. We generally expect
everything to be traced, so this shouldn't impact us.
==COMMIT_MSG==
Trace threadlocal entry is never removed, only nulled
==COMMIT_MSG==
Benchmarks on a single thread:
BEFORE
AFTER
Benchmarks with 20 threads
20 threads BEFORE
20 threads AFTER
GCProfiler benchmarks (gc profiling appears to impact the results)
Before:
After: