ppkarwasz commented on code in PR #2256:
URL: https://github.com/apache/logging-log4j2/pull/2256#discussion_r1470805337


##########
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:
   What about:
   ```java
   this(new HashMap<>(map), false);
   ```
   you proposed in #2101?
   
   Now that I removed all references to this constructor in **our** code, I 
don't have objections about it generating garbage. We could even use:
   ```java
   this(map, true);
   ```
   
   I think in the original #2098 report the user didn't implement 
`ContextDataProvider#supplyStringMap` but used the default method.



-- 
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]

Reply via email to