You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "kilink (via GitHub)" <gi...@apache.org> on 2025/08/01 17:23:51 UTC
[PR] Fix sizing of Maps [logging-log4j2]
kilink opened a new pull request, #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868
Fix several instances where Maps were sized incorrectly when the number of elements to insert is known; specifying the capacity without taking into account the load factor meant it would always result in a rehash / resize operation.
To this end, add a simple Maps utility class that handles the capacity calculation, much like the static HashMap.newHashMap helper that was added in JDK19.
## Checklist
Before we can review and merge your changes, please go through the checklist below. If you're still working on some items, feel free to submit your pull request as a draft—our CI will help guide you through the remaining steps.
### ✅ Required checks
- [ X ] **License**: I confirm that my changes are submitted under the [Apache License, Version 2.0](https://apache.org/licenses/LICENSE-2.0).
- [ X ] **Commit signatures**: All commits are signed and verifiable. (See [GitHub Docs on Commit Signature Verification](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)).
- [ X ] **Code formatting**: The code is formatted according to the project’s style guide.
- [ X ] **Build & Test**: I verified that the project builds and all unit tests pass.
<details>
<summary>How to build the project</summary>
Run: `./mvnw verify`
See [the build instructions](https://logging.apache.org/log4j/2.x/development.html#building) for details.
</details>
### 🧪 Tests (select one)
- [ ] I have added or updated tests to cover my changes.
- [ X ] No additional tests are needed for this change.
### 📝 Changelog (select one)
- [ ] I added a changelog entry in `src/changelog/.2.x.x`. (See [Changelog Entry File Guide](https://logging.apache.org/log4j/tools/log4j-changelog.html#changelog-entry-file)).
- [ X ] This is a trivial change and does not require a changelog entry.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2269725334
##########
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:
The `internal` packages are not for users, but for us: BND Baseline will not complain if we remove the class and we'll not address any users complains, since they have been sufficiently warned.
I am for merging this one, if my comment above is addressed.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2269301265
##########
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:
Since Log4j 2 supports Java 8, we don’t really have the concept of internal packages — everything is effectively available for use. Once we introduce a public class, it’s unlikely we’ll be able to remove it later (see [Hyrum’s Law](https://www.hyrumslaw.com/)).
I’m generally not in favor of optimizing `HashMap` resizing unless there’s a clear benefit, with the exception of `CloseableThreadContext`, where a simple inline math operation achieves the goal cleanly.
In short, I’m leaning `-0` on the PR in its current state. Your perspective is just as important as mine, so I’m happy to leave the final call to you.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3146535919
Thanks so much for the patch. Would you mind sharing some context, please?
Did you observe a particular performance issue?
> Message ID: ***@***.***>
>
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "kilink (via GitHub)" <gi...@apache.org>.
kilink commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2271168844
##########
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:
I've made the changes suggested by @ppkarwasz, thanks for the discussion / review.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3158233831
<table>
<tr>
<th>Job</th>
<th>Requested goals</th>
<th>Build Tool Version</th>
<th>Build Outcome</th>
<th>Build Scan®</th>
</tr>
<tr>
<td>build-macos-latest</td>
<td>clean install</td>
<td align='center'>3.9.8</td>
<td align='center'>:x:</td>
<td><a href="https://develocity.apache.org/s/byl56z6p43m5i" rel="nofollow"><img src="https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle" alt="Build Scan PUBLISHED" /></a></td>
</tr>
<tr>
<td>build-ubuntu-latest</td>
<td>clean install</td>
<td align='center'>3.9.8</td>
<td align='center'>:x:</td>
<td><a href="https://develocity.apache.org/s/zjbx4ragsot4s" rel="nofollow"><img src="https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle" alt="Build Scan PUBLISHED" /></a></td>
</tr>
<tr>
<td>build-windows-latest</td>
<td>clean install</td>
<td align='center'>3.9.8</td>
<td align='center'>:x:</td>
<td><a href="https://develocity.apache.org/s/5jum7dg6gltem" rel="nofollow"><img src="https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle" alt="Build Scan PUBLISHED" /></a></td>
</tr>
</table>
###### Generated by gradle/develocity-actions
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3159596085
I'm against enhancements without convincing figures obtained from benchmarks mapping to real-world use cases. If this optimization yields a tangible benefit on the hot path, provide a reproducer please.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2264564276
##########
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:
I've reviewed all `log4j-api` and `log4j-core` changes, and found this to be only change that one _might_ claim to be of importance. I prefer a PR that
1. Replaces all `HashMap#new(int)` calls with a `HashMap#new()` (Since they're all incorrectly implemented, it is better to just stick to defaults.)
2. Replaces only this line in `CloseableThreadContext` with `new HashMap((int) Math.ceil(originalValues.size() / 0.75))`
@ppkarwasz, WDYT?
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2265377992
##########
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:
Can you point out an example, where the usage of `HashMap#new(int)` is inappropriate?
I am mostly +1 on the proposed changes, as long as they don't introduce a new API. We have already dealt with methods and classes backported from newer JDK versions.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2265834766
##########
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:
> Can you point out an example, where the usage of `HashMap#new(int)` is inappropriate?
I don't know what you mean by _"inappropriate"_. All changes this PR highlights show `HashMap#new(int)` usages where the author of the associated piece of code _incorrectly_ assumed the provided capacity would avoid allocations, but it does not. Since, with the exception of `CloseableThreadContext`, all such usages are not on the hot path, it is better to not provide any capacity at all (i.e., remove the incorrect assumption), and just rely on Java standard library defaults.
> I am mostly +1 on the proposed changes, as long as they don't introduce a new API. We have already dealt with methods and classes backported from newer JDK versions.
I'm mostly -1 for adding changes optimizing code that that is not on a 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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2268739888
##########
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:
> * 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).
That is a fair path forward. This effectively translates to only fixing the `CloseableThreadContext` with an inline math operation, @ppkarwasz, do you agree?
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3158723587
Hi @kilink,
Adding a new utility class to `log4j-api` and then using it in `log4j-core` reinforces an unnatural dependency between the two modules, and would introduce a hard incompatibility between Log4j Core `2.26.0` and Log4j API `2.25.x`. The Core module should implement the API, not depend on it for internal utilities. I realize this kind of coupling already exists in some places, but I’d prefer not to increase it.
Could you instead:
* Move `Maps` to `o.a.l.l.util.internal` to make it clear it’s not intended for external use. While `o.a.l.l.util` is *theoretically* internal, it remains exported due to past API leaks (e.g., `o.a.l.l.util.Supplier`).
* Create a separate `o.a.l.l.core.util.internal.Maps` helper for `log4j-core`, so it can be used independently without introducing additional cross-module dependencies.
I agree with @vy that, without clear benchmark results, the value of this enhancement is questionable. We avoid using `HashMap` in performance-critical paths (such as the logging context map) because its instantiation and operation costs are high for small maps. Instead, we use `o.a.l.l.util.StringMap`, which offers enhanced features like mutability control (`freeze()`) and a `foreach` method (introduced for `Map` only in JDK 8). However, `StringMap` represents a significant technical debt we would like to eliminate. If you are interested in map performance, could you explore the feasibility of replacing `StringMap` with `Map` in version 3.x?
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2268805528
##########
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:
I’d prefer to replace it everywhere. My reasoning is:
These changes simply swap a constructor call for a static helper call.
If `log4j-api` or (more likely) `log4j-core` eventually moves to JDK 21, and we’ve only inlined the fix in a single spot, it’s very likely we’ll forget about it. On the other hand, if we use a `Maps` helper class, placed in an **internal** package per artifact, and document that it should be replaced with `HashMap.newHashMap` once our baseline is JDK 19+, there’s a much better chance we’ll catch it.
As I mentioned in https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3158723587, the only change I’d like to see in this PR is to hide the helper in an `internal` package that is **not** exported via OSGi or JPMS, so we can drop it cleanly when our baseline surpasses JDK 19. This obviously also means that `log4j-api` and `log4j-core` need a separate copy of the helper.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#discussion_r2266063976
##########
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:
> All changes this PR highlights show `HashMap#new(int)` usages where the author of the associated piece of code _incorrectly_ assumed the provided capacity would avoid allocations, but it does not.
I must be tired, but could you point me to some examples? I checked a couple of usages and, if the capacity of the map is computed correctly, they don't seem to cause the map to be resized.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "kilink (via GitHub)" <gi...@apache.org>.
kilink commented on PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3160865030
I personally don't think it's worthwhile to spend time writing a benchmark for what amounts to a bug fix / correctness issue. If you'd rather not accept the change, feel free to close the issue. I'll leave my rationale for fixing it however:
- Without a benchmark, we can see that the incorrect supplied capacity will cause a resize / rehash, and result in a map that is over-sized for the number of items inserted. It's easy to avoid with a trivial calculation which takes into account the load factor.
- It's true that not all instances of this I changed in the PR are likely to be on the hot path, but fixing it everywhere is more of a consistency thing; if the helper is there, why not, otherwise it would just be more confusing.
- There are likely callers in the wild that at least would be hitting the `toMap` implementation of the `ReadOnlyStringMap`, e.g. likely through `ThreadContextMap` (I can confirm such instances internally, as well as in OSS projects such as Sentry).
- Whoever wrote those lines of code was attempting to size the HashMaps precisely by passing in a capacity, it just so happens that the number of items to insert != the map capacity, so seems like an easy mistake to have made. I assume care was taken here though since log4j generally tries to be optimal in terms of allocations and performance.
I assume the main point of contention is the addition of a utility class, and I agree it is ugly, but it's a stopgap until (?) Log4j can use the static `HashMap.newHashMap` added in Java 19, I presume when the baseline JDK version log4j requires allows us to use that, we can just switch to it and remove any such shim utility class. Or another way to put it, I presume you would readily accept this change if it were simply delegating to the JDK helpers (assuming we could use it)?
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix sizing of Maps [logging-log4j2]
Posted by "kilink (via GitHub)" <gi...@apache.org>.
kilink commented on PR #3868:
URL: https://github.com/apache/logging-log4j2/pull/3868#issuecomment-3156023602
> Did you observe a particular performance issue?
I don't recall how I stumbled upon it or if it showed up in any performance profiles (it definitely has for other libraries). It's just a common thing I've noticed across various projects (e.g., spring and grpc-java) since the HashMap constructor is a foot gun and it's not easy to do correctly until Java 19. Since the intent here was to presize the map, and we know how many elements to insert, it just seemed to make sense to size it so we always avoid the resize / rehash operations.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org