On Sun, 20 Feb 2022 19:08:39 GMT, liach <d...@openjdk.java.net> wrote:

>>> @liach implementations `size()` seems O1, and returns a single int number 
>>> field, but it actually defers in some Map implementations. 
>> 
>> @liach for example, in ConcurrentSkipListMap and ConcurrentHashMap, `size()` 
>> function is far complicated than reading a field, thus calling it twice 
>> meaninglessly is not a wise choice.
>
> Imo you should just remove the `if (expectedSize == 0)` check than using this 
> somewhat ugly trick to avoid calling `size()` twice (the second call is only 
> used for this relatively useless fast-path, especially for the concurrent 
> collections you refer to)

In fact, if we do worry about the performance of adding from maps, calling 
`map.forEach(this::put);` would be a better alternative both in concurrency (as 
the concurrent map itself takes charage) and object allocation-wise (no 
allocation of immutable entry objects), but that belongs to another issue.

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

PR: https://git.openjdk.java.net/jdk/pull/7431

Reply via email to