ppkarwasz commented on PR #2256: URL: https://github.com/apache/logging-log4j2/pull/2256#issuecomment-1920033522
@rgoers, > This seems strange to me. Why wouldn't you create an ImmutableJdkMapAdapterStringMap that extends JdkMapAdapterStringMap (or vice versa) instead of using a boolean parameter? It seems like it would be cleaner. The original problem in #2098 was that a user implemented [`ContextDataProvider#supplyContextData`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/util/ContextDataProvider.html) using `Map#of`. This caused the generation of a `StringMap` that wasn't frozen, but wasn't modifiable either, since the underlying map was immutable. I tried auto-detecting the problem with the performance issue we are faced today. By adding a new constructor I want to give users clear directions that: - if the provided map is not immutable, Log4j might reuse it to prevent the allocation of new instances, - if the provided map is immutable, it must be marked so. I can create an `ImmutableJdkMapAdapterStringMap`, but then the contract of the original `JdkMapAdapterStringMap` constructor must be changed. What do you think? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
