On Wed, 24 Jan 2024 00:26:09 GMT, Joshua Cao <d...@openjdk.org> wrote:

> This change mirrors what we did for ConcurrentHashMap in 
> https://github.com/openjdk/jdk/pull/17116. When we add all entries from one 
> map to anther, we should resize that map to the size of the sum of both maps.
> 
> I used the command below to run the benchmarks. I set a high heap to reduce 
> garbage collection noise.
> 
> java -Xms25G -jar benchmarks.jar -p size=100000 -p addSize=100000 -gc true 
> org.openjdk.bench.java.util.HashMapBench
> 
> 
> Before change
> 
> 
> Benchmark            (addSize)        (mapType)  (size)  Mode  Cnt   Score   
> Error  Units
> HashMapBench.putAll     100000         HASH_MAP  100000  avgt    4  22.927 ± 
> 3.170  ms/op
> HashMapBench.putAll     100000  LINKED_HASH_MAP  100000  avgt    4  25.198 ± 
> 2.189  ms/op
> 
> 
> After change
> 
> 
> Benchmark            (addSize)        (mapType)  (size)  Mode  Cnt   Score   
> Error  Units
> HashMapBench.putAll     100000         HASH_MAP  100000  avgt    4  16.780 ± 
> 0.526  ms/op
> HashMapBench.putAll     100000  LINKED_HASH_MAP  100000  avgt    4  19.721 ± 
> 0.349  ms/op
> 
> 
> We see about average time improvements of 26% in HashMap and 20% in 
> LinkedHashMap.

Then we might need some statistics on how often `putAll` replaces existing 
mappings, ranging from none at all to completely. For example, 
`Collectors.toMap` would never replace existing mappings, even though it 
doesn't use `putAll` (it can probably call putAll and throw exception if the 
new size isn't 2 old sizes added up)

The current allocation assumes putting replaces all existing mappings, which I 
don't think is quite applicable.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17544#issuecomment-1908819980

Reply via email to