On Mon, Dec 30, 2019 at 10:15 PM Carter Kozak <[email protected]> wrote:
> Beyond StringBuilder instances, we attempt to clear references
> from all thread local references to avoid substantial overhead. In
> practice this works nicely because it reinforces java performance
> characteristics. Java threads are fairly memory expensive (not to
> mention the cost of initialization) so the threadlocal object overhead
> from log4j tends to be inconsequential by comparison. Applications
> in memory constrained environments already have relatively few
> threads, and applications which constantly create and destroy threads
> tend not to worry about the performance of creating log events
> because it's inexpensive compared to thread initialization.
>
> Have you observed a problem? We've found and resolved a few issues
> over the last year or so where references were held longer than
> expected. If you're aware of places we're using more memory than we
> should, please file a ticket :-)
AFAIC, the only TLA in Log4j 2.0 core violating
log4j2.enableThreadlocals flag is
AbstractStringLayout#getStringBuilder(). Given AbstractStringLayout is
used by many internal (HTML, XML, JSON, YAML, Pattern, Gelf, Syslog)
and external (ECS) layouts, the fix will incur a significant
performance penalty. I wouldn't be surprised if we start receiving
performance regression bug reports from users after releasing such a
fix, since a notable amount of Log4j 2.0 users, to the best of my
knowledge, are using it in JEE context (e.g., Spring WebMvc with
Tomcat backend) where ENABLE_THREADLOCALS are disabled due to the
present IS_WEB_APP condition:
o.a.l.l.u.Constants.ENABLE_THREADLOCALS =
!IS_WEB_APP &&
PropertiesUtil
.getProperties()
.getBooleanProperty("log4j2.enable.threadlocals", true);
Created LOG4J2-2753[1] for this issue.
The reason I started the discussion is, in log4j2-logstash-layout, I
am aiming for the fastest approach, always. The performance comparison
is JMH-driven, where all competitors (LogstashLayout, EcsLayout,
JsonLayout, etc.) are fine tuned for fairness. There I try to play
fair, but neither Log4j 2.0 JsonLayout, nor EcsLayout of Elastic does
that; they perform TLA without taking log4j2.enableThreadlocals flag
into account. This leads me to questionthe presence of such a flag in
the first place. Why don't we just remove the
log4j2.enableThreadlocals flag? What are its use cases?
[1] https://issues.apache.org/jira/browse/LOG4J2-2753
