Would it be useful to implement some sort of buffer pool for
StringBuilders and ByteBuffers? Could likely copy code from netty's
util library (ByteBuf et al.) or reuse stuff from commons-pool if
needed. This would work properly in applications, servlets, and even
reactive streams and lightweight threads later on.
On Tue, 31 Dec 2019 at 03:22, Volkan Yazıcı <volkan.yaz...@gmail.com> wrote:
>
> On Mon, Dec 30, 2019 at 10:15 PM Carter Kozak <cko...@ckozak.net> 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
--
Matt Sicker <boa...@gmail.com>
