Re: Release timeline for 2.24.0
I'd like to have the PMC meet to review and discuss ScopedContext, which I am not caught up on, and whatever else we should chat about. Gary On Fri, May 24, 2024 at 2:11 PM Piotr P. Karwasz wrote: > > Hi John, > > On Fri, 24 May 2024 at 17:17, John Engebretson wrote: > > The long-debated ScopedContext is not significant to us. Can > > we set a target date for 2.24.0? Perhaps by punting ScopedContext to a > > later date? > > +1, the release of 2.24.0 is a blocker for: > > * the release of a performance-optimized and web-app safe > `ThreadContextMap` you contributed more than 2 months ago[1], > * the release of the next 3.0.0 beta, which will be the first without > a `log4j-api` artifact, > * a Log4j API without the LambdaMetaFactory call that makes versions > 2.19.0 to 2.23.1 unsuitable for GraalVM. > > Probably by the beginning of June (after CoCEU 2024) I can: > > 1. Move `ScopedContext` to branch 2.25.x, > 2. Replace `DefaultThreadContextMap` with the new > `StringArrayThreadContextMap`, > 3. Check if `StringArrayThreadContextMap` allows for "garbage-free > logging" (if it doesn't, it must be very close) and remove > `CopyOnWriteThreadContextMap`, > > So realistically we could expect 2.24.0 by the end of June. > > I believe that ScopedContext is a valid idea, but it still requires > some massaging. I haven't seen any performance figures yet and if the > main selling point is "you can't forget to clean up the thread > context", I am not too convinced of its utility. ThreadContext and MDC > have been around for a long time and people know how to use them > correctly. > > We should probably: > > * have a PMC meeting to brainstorm on all the selling points of > `ScopedContext`, > * do some benchmarks to see its performance, > * review all its methods so we don't repeat the errors of the past. As > I stated several times, 'ThreadContext.get()` is a mistake, because it > allows users to abuse the API and transport logging-unrelated data > through `ThreadContext`. If we could correct the past, I would remove > `ThreadContext.get()` and let `ThreadContext.put()` return the old > value. > > Piotr > > [1] https://github.com/apache/logging-log4j2/pull/2330
Re: Release timeline for 2.24.0
Hi Ralph, On Fri, 24 May 2024 at 22:50, Ralph Goers wrote: > Thanks! Is there any chance you could provide some sample code we could use > to perform performance testing? Based on your description I have a suspicion > that ScopedContext would perform even better but at this point that is just > speculation without tests. Of course, that may be minimal and might not be > worth the effort of having to change code. I am sure `ScopedContext` can be as fast as `ThreadContext`. I pushed a JMH benchmark we could improve to get some measurements[1]. It's a work in progress, I am not sharing the first results, because the two differ by almost two orders of magnitude. I suspect some aggressive JVM optimization is taking place. Piotr [1] https://github.com/apache/logging-log4j2/blob/2.x/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/jmh/ThreadContextVsScopedContextBenchmark.java
Re: Release timeline for 2.24.0
Thanks! Is there any chance you could provide some sample code we could use to perform performance testing? Based on your description I have a suspicion that ScopedContext would perform even better but at this point that is just speculation without tests. Of course, that may be minimal and might not be worth the effort of having to change code. You say that it is mostly for a logging/metrics library. Does that mean it is storing counters that it has to periodically increment? I’m curious there since the MDC/ThreadContextMap only support strings. Ralph > On May 24, 2024, at 12:00 PM, John Engebretson wrote: > > Certainly! Our usage is widespread in our applications, and mostly > wrapped inside an internal logging/metrics library. Focusing on our most > performance-critical applications, our usage falls into two categories: > > 1. MDC sets/gets, ranging from 1 to 8 values at any given time. > 2. CloseableThreadContext, with attribute counts varying from 0 to 15 > values at any given time. > > Our performance profiling/investigation/tooling highlighted the cost of > HashMap operations when modifying the context; that cost involved cpu time, > critical-path clock time, and memory allocation/GC. The largest amount of > work comes from copying the old HashMap into the new one - iterating across > empty buckets, creating new Nodes, recalculating hashcodes, etc. > 2.24.0 will reduce the runtime cost with no application changes. We like > that outcome. :) >John > > On Fri, May 24, 2024 at 1:47 PM Ralph Goers > wrote: > >> >> >>> On May 24, 2024, at 11:10 AM, Piotr P. Karwasz >> wrote: >>> >>> Hi John, >>> >>> On Fri, 24 May 2024 at 17:17, John Engebretson >> wrote: As >>> I stated several times, 'ThreadContext.get()` is a mistake, because it >>> allows users to abuse the API and transport logging-unrelated data >>> through `ThreadContext`. If we could correct the past, I would remove >>> `ThreadContext.get()` and let `ThreadContext.put()` return the old >>> value. >> >> John, >> >> ThreadContextMap is obviously very important to your use case. Can you >> describe how you are using it? What about the way you are using it made you >> notice it was having an impact on performance? >> >> Ralph
Re: Release timeline for 2.24.0
Certainly! Our usage is widespread in our applications, and mostly wrapped inside an internal logging/metrics library. Focusing on our most performance-critical applications, our usage falls into two categories: 1. MDC sets/gets, ranging from 1 to 8 values at any given time. 2. CloseableThreadContext, with attribute counts varying from 0 to 15 values at any given time. Our performance profiling/investigation/tooling highlighted the cost of HashMap operations when modifying the context; that cost involved cpu time, critical-path clock time, and memory allocation/GC. The largest amount of work comes from copying the old HashMap into the new one - iterating across empty buckets, creating new Nodes, recalculating hashcodes, etc. 2.24.0 will reduce the runtime cost with no application changes. We like that outcome. :) John On Fri, May 24, 2024 at 1:47 PM Ralph Goers wrote: > > > > On May 24, 2024, at 11:10 AM, Piotr P. Karwasz > wrote: > > > > Hi John, > > > > On Fri, 24 May 2024 at 17:17, John Engebretson > wrote: > >> As > > I stated several times, 'ThreadContext.get()` is a mistake, because it > > allows users to abuse the API and transport logging-unrelated data > > through `ThreadContext`. If we could correct the past, I would remove > > `ThreadContext.get()` and let `ThreadContext.put()` return the old > > value. > > John, > > ThreadContextMap is obviously very important to your use case. Can you > describe how you are using it? What about the way you are using it made you > notice it was having an impact on performance? > > Ralph
Re: Release timeline for 2.24.0
> On May 24, 2024, at 11:10 AM, Piotr P. Karwasz > wrote: > > Hi John, > > On Fri, 24 May 2024 at 17:17, John Engebretson wrote: >> As > I stated several times, 'ThreadContext.get()` is a mistake, because it > allows users to abuse the API and transport logging-unrelated data > through `ThreadContext`. If we could correct the past, I would remove > `ThreadContext.get()` and let `ThreadContext.put()` return the old > value. John, ThreadContextMap is obviously very important to your use case. Can you describe how you are using it? What about the way you are using it made you notice it was having an impact on performance? Ralph
Re: Release timeline for 2.24.0
Hi John, On Fri, 24 May 2024 at 17:17, John Engebretson wrote: > The long-debated ScopedContext is not significant to us. Can > we set a target date for 2.24.0? Perhaps by punting ScopedContext to a > later date? +1, the release of 2.24.0 is a blocker for: * the release of a performance-optimized and web-app safe `ThreadContextMap` you contributed more than 2 months ago[1], * the release of the next 3.0.0 beta, which will be the first without a `log4j-api` artifact, * a Log4j API without the LambdaMetaFactory call that makes versions 2.19.0 to 2.23.1 unsuitable for GraalVM. Probably by the beginning of June (after CoCEU 2024) I can: 1. Move `ScopedContext` to branch 2.25.x, 2. Replace `DefaultThreadContextMap` with the new `StringArrayThreadContextMap`, 3. Check if `StringArrayThreadContextMap` allows for "garbage-free logging" (if it doesn't, it must be very close) and remove `CopyOnWriteThreadContextMap`, So realistically we could expect 2.24.0 by the end of June. I believe that ScopedContext is a valid idea, but it still requires some massaging. I haven't seen any performance figures yet and if the main selling point is "you can't forget to clean up the thread context", I am not too convinced of its utility. ThreadContext and MDC have been around for a long time and people know how to use them correctly. We should probably: * have a PMC meeting to brainstorm on all the selling points of `ScopedContext`, * do some benchmarks to see its performance, * review all its methods so we don't repeat the errors of the past. As I stated several times, 'ThreadContext.get()` is a mistake, because it allows users to abuse the API and transport logging-unrelated data through `ThreadContext`. If we could correct the past, I would remove `ThreadContext.get()` and let `ThreadContext.put()` return the old value. Piotr [1] https://github.com/apache/logging-log4j2/pull/2330
Release timeline for 2.24.0
Hi, my company is looking forward to the performance improvements associated with 2.24.0 (specifically #2330). Most of our applications rely on MDC or CloseableThreadContext and the performance benefits will be widespread. The long-debated ScopedContext is not significant to us. Can we set a target date for 2.24.0? Perhaps by punting ScopedContext to a later date? Thanks! John
