ppkarwasz commented on code in PR #2256:
URL: https://github.com/apache/logging-log4j2/pull/2256#discussion_r1470938012
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java:
##########
@@ -47,9 +47,14 @@ public class JdkMapAdapterStringMap implements StringMap {
private transient String[] sortedKeys;
public JdkMapAdapterStringMap() {
- this(new HashMap<String, String>());
+ this(new HashMap<String, String>(), false);
}
+ /**
+ * @deprecated for performance reasons since 2.23.
+ * Use {@link #JdkMapAdapterStringMap(Map, boolean)} instead.
+ */
+ @Deprecated
public JdkMapAdapterStringMap(final Map<String, String> map) {
this.map = Objects.requireNonNull(map, "map");
try {
Review Comment:
Anything is faster than catching an exception. ;-)
I like the `HashMap::new` version better, because it enforces semantics:
IMHO a **frozen** `StringMap` should be immutable, while a `StringMap` created
from a `Collections.unmodifiableMap` can be modified by changing the underlying
map.
As I remarked, I removed all references to the deprecated constructor in our
code. On Github I could only find two other projects that use
`JdkMapAdapterStringMap`:
-
[zenmoto/opentelemetry_logging_prototypes](https://github.com/zenmoto/opentelemetry_logging_prototypes/blob/main/log4j/src/main/java/io/opentelemetry/sdk/proto/LayoutWrapper.java),
-
[apple/servicetalk](https://github.com/apple/servicetalk/blob/main/servicetalk-opentracing-log4j2/src/main/java/io/servicetalk/opentracing/log4j2/ServiceTalkTracingThreadContextMap.java).
--
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]
