vy commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2266818554
##########
log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java:
##########
@@ -206,7 +207,7 @@ public void close() {
}
private void closeMap() {
- final Map<String, String> valuesToReplace = new
HashMap<>(originalValues.size());
+ final Map<String, String> valuesToReplace =
Maps.newHashMap(originalValues.size());
Review Comment:
For one, all `*Test` suggestions are redundant. The rest of the usages are
incorrect, see `ThreadDumpMessage`:
```
Map<ThreadInformation, StackTraceElement[]> threads = new
HashMap<>(map.size());
for (Map.Entry<Thread, StackTraceElement[]> entry : map.entrySet()) {
threads.put(...);
}
```
Author had the intention of creating an initial `HashMap` of capacity
`map.size()` to avoid resizing.
1. This is _incorrect_, since it _will be_ resized at the .75 mark
2. This code is hardly on a hot path, if used at all
I'm not saying what this PR implements is _incorrect_, instead, what is
already there in the existing code base is incorrect. Plus, there is nothing to
be corrected if just the defaults are used. Being extra smart does not buy
anything but complicate the code.
--
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
