I am not against making a change but I would need to see performance results before it gets merged.
Ralph > On Jan 6, 2020, at 2:20 PM, Carter Kozak <cko...@ckozak.net> wrote: > > 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> >>