RFR: 8322149: ConcurrentHashMap copy constructor should not transfer from old table on presizing [v2]

[Volker Simonis]

On Wed, 3 Jan 2024 21:29:57 GMT, Joshua Cao <duke at openjdk.org> wrote:

>> ConcurrentHashMap's copy constructor calls `putAll()` -> `tryPresize()` -> `transfer()`. When coming from the copy constructor, the Map is empty, so there is nothing to transfer. But `transfer()` will still copy all the empty nodes from the old table to the new table.
>> 
>> This patch avoids this work by only calling `tryPresize()` if the table is already initialized. If `table` is null, the initialization is deferred to `putVal()`, which calls `initTable()`.
>> 
>> ---
>> 
>> ### JMH results for testCopyConstructor
>> 
>> before patch:
>> 
>> 
>> Result "org.openjdk.bench.java.util.concurrent.Maps.testCopyConstructor":
>>   937395.686 ±(99.9%) 99074.324 ns/op [Average]
>>   (min, avg, max) = (825732.550, 937395.686, 1072024.041), stdev = 92674.184
>>   CI (99.9%): [838321.362, 1036470.010] (assumes normal distribution)
>> 
>> 
>> after patch:
>> 
>> 
>> Result "org.openjdk.bench.java.util.concurrent.Maps.testCopyConstructor":
>>   620871.469 ±(99.9%) 59195.406 ns/op [Average]
>>   (min, avg, max) = (545304.633, 620871.469, 689013.573), stdev = 55371.419
>>   CI (99.9%): [561676.063, 680066.875] (assumes normal distribution)
>> 
>> 
>> Average time is decreased by about 33%.
>
> Joshua Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cleanup benchmark

src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java line 1088:

> 1086:     public void putAll(Map<? extends K, ? extends V> m) {
> 1087:         if (table != null) {
> 1088:             tryPresize(m.size());

Shouldn't this be something like `tryPresize(this.size() + m.size())` to accommodate for the worst case where there are no common keys in `this` map and `m`?

Or do we intentionally try to be  conservative with memory usage? But then shouldn't we use at least `max(this.size(), m.size)`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17116#discussion_r1444898430