Move JCTools-based queues to a new log4j-jctools module
#2104
Conversation
ppkarwasz
left a comment
ppkarwasz
left a comment
There was a problem hiding this comment.
Nice job, looks good to me.
We might reevaluate the need for alternative recycler implementations in log4j-api in the future, but for now I agree that log4j-core is all we need.
I start to wonder how useful is it to fill log4j-api with partial implementations. For example a recycled LogBuilder is useful for log4j-core, but not so much for log4j-to-slf4j. If log4j-to-slf4j is bound to Logback, the Log4j API builder will be recycled, but the underlying SLF4J builder won't. We might as well remove Recycler from AbstractLogger and move it into core.Logger.
| /** | ||
| * Denotes the value to be used while sorting recycler factory providers to determine the precedence order. | ||
| * Values will be sorted naturally, that is, lower values will imply higher precedence. | ||
| * | ||
| * @return the value to be used while sorting | ||
| */ | ||
| default int getOrder() { | ||
| return 100; | ||
| } | ||
|
|
||
| /** | ||
| * The name of this recycler factory provider. | ||
| * Recycler factory providers are required to have unique names. | ||
| * | ||
| * @return the name of this recycler factory provider | ||
| */ | ||
| String getName(); |
There was a problem hiding this comment.
In the future we might consider using ServiceLoader.Provider<RecyclerFactory> instead of this class and add an @Named and @Order annotation to the implementations of RecyclerFactory. We already have a @Named and @Order annotation in log4j-core and log4j-plugins. @jvz, what do you think?
I could open an issue to explore this direction, although I don't have too much confidence in ServiceLoader#stream, which is the only way to obtain ServiceLoader.Provider instances: it is unable to skip faulty service implementations.
log4j-api/src/main/java/org/apache/logging/log4j/spi/recycler/package-info.java
Outdated
Show resolved
Hide resolved
...main/java/org/apache/logging/log4j/internal/recycler/ThreadLocalRecyclerFactoryProvider.java
Show resolved
Hide resolved
Co-authored-by: Piotr P. Karwasz <[email protected]>
|
2 participants
This PR addresses #2075 by means of following changes:
ServiceProviderslog4j-apiandlog4j-coreare removed in favor of a new module:log4j-jctoolsWhy does
log4j-jctoolsdepend onlog4j-core?There are two places where JCTools is used as an optional dependency:
log4j-apias aRecycler[Factory](used almost everywhere) implementationlog4j-coreas aBlockingQueueFactory(used byAsyncAppenderinlog4j-core) implementationThere are two approaches I can think of to remove the JCTools optional dependencies:
BlockingQueueFactoryfromlog4j-coretolog4j-apilog4j-jctoolswill only need to depend onlog4j-apilog4j-apiwill be introduced a component that only matters for somelog4j-coreinternallog4j-corehas access to plugins and hence, users were expected to provide custom@Configurable @Plugin BlockingQueueFactoryimplementations that they can refer to inlog4j2.xml. Though movingBlockingQueueFactorytolog4j-apiwill break this contract completely, sincelog4j-apihas no notion of plugins.log4j-jctoolsdepend onlog4j-coreand provide bothRecycler[Factory]andBlockingQueueFactoryimplementationslog4j-coreet al. (if there is such a use case at all) needs to manually excludelog4j-coreReview kit
o.a.l.log4j.internal.recyclerando.a.l.log4j.spipackages oflog4j-apimodulelog4j-corelog4j-jctools