ppkarwasz commented on PR #2593: URL: https://github.com/apache/logging-log4j2/pull/2593#issuecomment-2126401153
> Question: With the changes I recently made are these alternative implementations really of value to anything? The primary motivation for them seemed to be to reduce the number of garbage objects being created or the amount of copying being done. But now the algorithm used no longer needs to copy the maps, which makes me wonder if they are even needed. You are right, maybe we can drop some implementations, but others must stay: * John's `StringArrayThreadContextMap` outperforms `DefaultThreadContextMap` and is web-application safe, even if the user forgets to clear the map at the end of processing! We could however **remove** `DefaultThreadContextMap` and rename `StringArrayThreadContextMap` to `DefaultThreadContextMap`, * `GarbageFreeSortedArrayThreadContextMap` should stay, since it is the only one that provides a provably garbage-free experience, when accessed through `ThreadContext`. Both `ScopedContext` and `CloseableThreadContext` generate temporary objects. We could rewrite it to use a mutable version of John's [UnmodifiableArrayBackedMap](https://github.com/apache/logging-log4j2/blob/b83699fad40561bfe3c1dfab32279af542160059/log4j-core/src/main/java/org/apache/logging/log4j/core/context/internal/UnmodifiableArrayBackedMap.java) though, * `CopyOnWriteSortedArrayThreadContextMap` can probably be **removed**, since Java 8 made `StringMap`-based implementations obsolete. In general I think that people will be using `ThreadContext` for a very long time, so we might as well improve its implementations. Sure the `ThreadContext` API is very error prone compared to `ScopedContext`, but so is the usage of locks vs synchronized blocks or `ThreadLocal` vs the new `ScopedValue`. Experienced programmers have been using those **correctly** for years. Anyway, the main point of this PR is to make `log4j-api` lighter: * the API started at 115 KiB (which already twice the size of the current `slf4j-api`), * by the time `2.7` was published, it reached 213 KiB, * release `2.17.1` has a size of 294 KiB, * release `2.23.1`: 334 KiB, * now we are oscillating around 372 KiB. Since only `log4j-core` uses our `ThreadContextMap` implementations (`log4j-to-slf4j` uses whatever Logback provides, `log4j-to-jul` uses No-op), we can move the implementations to `log4j-core`, where we can publish a major version every couple of years. In Log4j API on the other hand we can not even remove the interfaces that we introduced so that Java 7 users can profit from Java 8 improvements. Those interfaces will be there long after Java 7 ceases to be used. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
