RFR: 8324573: HashMap::putAll should resize to sum of both map sizes
Chen Liang
liach at openjdk.org
Wed Jan 24 19:58:26 UTC 2024
On Wed, 24 Jan 2024 00:26:09 GMT, Joshua Cao <duke at 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
More information about the core-libs-dev
mailing list