ppkarwasz commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2267247024
##########
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
Thank you for clarifying! I initially thought your comment *“Since they're
all incorrectly implemented, it is better to just stick to defaults”* referred
to something else (e.g., off-by-one errors), rather than the specific confusion
between `HashMap`’s **capacity** and **threshold** that this PR addresses.
Given that, I’m even less clear on why you would suggest replacing
`HashMap::new(int)` with `HashMap::new()`. Since we’re already discussing this,
why not either:
* Keep the current code **unchanged**, accepting that it may trigger a
resize for some values of n (but not all, since the threshold is always 0.75 ×
the smallest power of 2 ≥ n).
* Apply the changes in this PR to correctly calculate the initial capacity
and avoid unintended resizes even for code that is not on the hot path.
--
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
