Conversation
`InstantPatternThreadLocalCachedFormatter` keyed its per-thread cache on a
`Long`: the epoch instant was extracted through a `Function<Instant, Long>`
and stored, boxed, into an `Object[]` held in a `ThreadLocal`. Both extractors,
`Instant::getEpochMillisecond` and `Instant::getEpochSecond`, return a
primitive, and epoch values are far outside the `Long` cache.
The extractor's box turns out to be scalar-replaced on a cache hit, since it
is unboxed straight into a local and never escapes. The box stored into the
`Object[]` does escape, so every cache miss allocated. A miss happens once per
distinct instant per thread, which for any thread logging at under a thousand
events per second is every single event.
Replace the extractor with a `ToLongFunction<Instant>` and the `Object[]` with
a small holder carrying a primitive `long` field next to the buffer.
`StringBuilderEncoder` documents a preference for keeping only JDK types in
thread locals, to avoid pinning a class loader in a web container, but that
does not apply here: this wrapper is only installed when
`Constants.ENABLE_THREADLOCALS` is set, and that flag is off for web
applications precisely to disable the thread locals which could leak.
Measured with the JMH benchmark added here, 1000 instants per operation:
before after
cacheMiss 24000.152 B/op, 153 GCs 0.141 B/op, no GC
cacheMiss 21744 +- 2404 ns/op 20274 +- 406 ns/op
cacheHit 0.038 B/op 0.040 B/op
cacheHit 5410 +- 70 ns/op 5696 +- 699 ns/op
Also close a cache-poisoning window while here: the cached epoch instant is
now cleared before the buffer is rewritten and restored only once formatting
succeeds, so a throwing formatter can no longer leave a later call reading a
half-written buffer. Note that merely moving the assignment after the
formatting does not fix this; it only changes which instant triggers it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vy
requested changes
Sep 16, 2026
vy
left a comment
Member
There was a problem hiding this comment.
This [seemingly AI-generated] content will need some review. I'm not very keen on adding yet another ThreadLocal to Log4j's event rendering pipeline. Allow me some time, please.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
InstantPatternThreadLocalCachedFormatterkeyed its per-thread cache on aLong: the epoch instant was extracted through aFunction<Instant, Long>and stored, boxed, into anObject[]held in aThreadLocal. Both extractors,Instant::getEpochMillisecondandInstant::getEpochSecond, return a primitive, and epoch values are far outside theLongcache.The extractor's box turns out to be scalar-replaced on a cache hit, since it is unboxed straight into a local and never escapes. The box stored into the
Object[]does escape, so every cache miss allocated. A miss happens once per distinct instant per thread, which for any thread logging at under a thousand events per second is every single event.Replace the extractor with a
ToLongFunction<Instant>and theObject[]with a small holder carrying a primitivelongfield next to the buffer.StringBuilderEncoderdocuments a preference for keeping only JDK types in thread locals, to avoid pinning a class loader in a web container, but that does not apply here: this wrapper is only installed whenConstants.ENABLE_THREADLOCALSis set, and that flag is off for web applications precisely to disable the thread locals which could leak.Measured with the JMH benchmark added here, 1000 instants per operation:
Also close a cache-poisoning window while here: the cached epoch instant is now cleared before the buffer is rewritten and restored only once formatting succeeds, so a throwing formatter can no longer leave a later call reading a half-written buffer. Note that merely moving the assignment after the formatting does not fix this; it only changes which instant triggers it.
INSERT HERE a clear and concise description of what the pull request is for along with a reference to the associated issue IDs, if they exist.