Let's give it a try and see how we like it :-) I probably won't have time to
prototype it for a month or two, but I think we could gain performance by
passing top level objects around (including between threads) instead of copying
data between buffer and objects on heap.
-ck
On Mon, Jan 6, 2020, at 14:17, Matt Sicker wrote:
> It seems like it'd be more flexible to tune, too. The current
> ThreadLocal approach scales uncontrollably with the number of threads
> as it is.
>
> On Mon, 6 Jan 2020 at 12:20, Gary Gregory <garydgreg...@gmail.com> wrote:
> >
> > I like the idea of centrally controlling these objects. This should make
> > resource monitoring easier as well.
> >
> > Gary
> >
> > On Mon, Jan 6, 2020, 13:09 Matt Sicker <boa...@gmail.com> wrote:
> >
> > > 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>
> > >
>
>
>
> --
> Matt Sicker <boa...@gmail.com>
>
