[Profiling] Reduce GC pressure #93590
Conversation
With this commit we reduce GC pressure in the profiling plugin by: * Presizing collections to the correct size. * Avoiding copying data needlessly by already using the collection type required by call sites.
danielmitterdorfer
added
>enhancement
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
|
Hi @danielmitterdorfer, I've created a changelog YAML for you. |
| this.executables = new ConcurrentHashMap<>(executableCount); | ||
| this.stackFrames = new ConcurrentHashMap<>(stackFrameCount); |
There was a problem hiding this comment.
You have to handle the "load facter" in maps.
You need to do something similar to org.elasticsearch.common.util.Maps here (I would say add a newConcurrentMapWithExpectedSize method there...)
static int capacity(int expectedSize) {
assert expectedSize >= 0;
return expectedSize < 2 ? expectedSize + 1 : (int) (expectedSize / 0.75 + 1.0);
}
There was a problem hiding this comment.
Thanks for the comment. I thought so as well but if you look at the constructor of ConcurrentHashMap that is not necessary. It is considering the load factor (0.75) already:
| if (j < capacity) { | ||
| Arrays.fill(output, j, capacity, 0); | ||
| } | ||
| return List.of(output); |
There was a problem hiding this comment.
If it being unmodifiable is important, use this. But Arrays.asList will probably do less copying as it will simply wrap the underlying array.
There was a problem hiding this comment.
Ah, good point. I've pushed e604f66.
| int slices | ||
| ) { | ||
| this.stackTracePerId = new ConcurrentHashMap<>(stackTraceCount); |
There was a problem hiding this comment.
You need to handle the map LoadFactor here.
|
Thanks for the review!
Yes, I did measurements. Admitted, the benefit is not huge: I see around 1% less young GCs with this change. However, we have tens of thousands of items to retrieve and we know the exact number upfront and to me this is a prime example where we should presize the respective collections instead of letting them grow. |
danielmitterdorfer
added
auto-backport-and-merge
With this commit we reduce GC pressure in the profiling plugin by: * Presizing collections to the correct size. * Avoiding copying data needlessly by already using the collection type required by call sites.
With this commit we reduce GC pressure in the profiling plugin by: * Presizing collections to the correct size. * Avoiding copying data needlessly by already using the collection type required by call sites.
With this commit we reduce GC pressure in the profiling plugin by: