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>
>> 


Reply via email to