On Thu, 17 Dec 2020 10:15:23 GMT, Сергей Цыпанов <github.com+10835776+stsypa...@openjdk.org> wrote:
>> Hello, I feel like this was previously discussed in >> https://mail.openjdk.java.net/pipermail/core-libs-dev/ but since I cannot >> find original mail I post this here. >> >> Currently `Collections.addAll()` is implemented and documented as: >> /** >> * ... >> * The behavior of this convenience method is identical to that of >> * {@code c.addAll(Arrays.asList(elements))}, but this method is likely >> * to run significantly faster under most implementations. >> */ >> @SafeVarargs >> public static <T> boolean addAll(Collection<? super T> c, T... elements) >> { >> boolean result = false; >> for (T element : elements) >> result |= c.add(element); >> return result; >> } >> >> But it practice the notation `this method is likely to run significantly >> faster under most implementations` is completely wrong. When I take this >> [benchmark](https://github.com/stsypanov/benchmarks/blob/master/benchmark-runners/src/main/java/com/luxoft/logeek/benchmark/collection/CollectionsAddAllVsAddAllBenchmark.java) >> and run it on JDK 14 I get the following results: >> (collection) (size) >> Score Error Units >> addAll ArrayList 10 >> 37.9 ± 1.9 ns/op >> addAll ArrayList 100 >> 83.8 ± 3.4 ns/op >> addAll ArrayList 1000 >> 678.2 ± 23.0 ns/op >> collectionsAddAll ArrayList 10 >> 50.9 ± 1.1 ns/op >> collectionsAddAll ArrayList 100 >> 751.4 ± 47.4 ns/op >> collectionsAddAll ArrayList 1000 >> 8839.8 ± 710.7 ns/op >> >> addAll HashSet 10 >> 128.4 ± 5.9 ns/op >> addAll HashSet 100 >> 1864.2 ± 102.4 ns/op >> addAll HashSet 1000 >> 16615.5 ± 1202.6 ns/op >> collectionsAddAll HashSet 10 >> 172.8 ± 6.0 ns/op >> collectionsAddAll HashSet 100 >> 2355.8 ± 195.4 ns/op >> collectionsAddAll HashSet 1000 >> 20364.7 ± 1164.0 ns/op >> >> addAll ArrayDeque 10 >> 54.0 ± 0.4 ns/op >> addAll ArrayDeque 100 >> 319.7 ± 2.5 ns/op >> addAll ArrayDeque 1000 >> 3176.9 ± 22.2 ns/op >> collectionsAddAll ArrayDeque 10 >> 66.5 ± 1.4 ns/op >> collectionsAddAll ArrayDeque 100 >> 808.1 ± 55.9 ns/op >> collectionsAddAll ArrayDeque 1000 >> 5639.6 ± 240.9 ns/op >> >> addAll CopyOnWriteArrayList 10 >> 18.0 ± 0.7 ns/op >> addAll CopyOnWriteArrayList 100 >> 39.4 ± 1.7 ns/op >> addAll CopyOnWriteArrayList 1000 >> 371.1 ± 17.0 ns/op >> collectionsAddAll CopyOnWriteArrayList 10 >> 251.9 ± 18.4 ns/op >> collectionsAddAll CopyOnWriteArrayList 100 >> 3405.9 ± 304.8 ns/op >> collectionsAddAll CopyOnWriteArrayList 1000 >> 247496.8 ± 23502.3 ns/op >> >> addAll ConcurrentLinkedDeque 10 >> 81.4 ± 2.8 ns/op >> addAll ConcurrentLinkedDeque 100 >> 609.1 ± 26.4 ns/op >> addAll ConcurrentLinkedDeque 1000 >> 4494.5 ± 219.3 ns/op >> collectionsAddAll ConcurrentLinkedDeque 10 >> 189.8 ± 2.5 ns/op >> collectionsAddAll ConcurrentLinkedDeque 100 >> 1660.0 ± 62.0 ns/op >> collectionsAddAll ConcurrentLinkedDeque 1000 >> 17649.2 ± 300.9 ns/op >> >> addAll:·gc.alloc.rate.norm ArrayList 10 >> 160.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm ArrayList 100 >> 880.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm ArrayList 1000 >> 8080.3 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ArrayList 10 >> 80.0 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ArrayList 100 >> 1400.2 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ArrayList 1000 >> 15025.6 ± 0.1 B/op >> >> addAll:·gc.alloc.rate.norm HashSet 10 >> 464.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm HashSet 100 >> 5328.5 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm HashSet 1000 >> 48516.7 ± 0.1 B/op >> collectionsAddAll:·gc.alloc.rate.norm HashSet 10 >> 464.0 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm HashSet 100 >> 5328.5 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm HashSet 1000 >> 48516.6 ± 0.1 B/op >> >> addAll:·gc.alloc.rate.norm ArrayDeque 10 >> 112.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm ArrayDeque 100 >> 560.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm ArrayDeque 1000 >> 4160.5 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ArrayDeque 10 >> 112.0 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ArrayDeque 100 >> 1048.1 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ArrayDeque 1000 >> 14929.4 ± 0.0 B/op >> >> addAll:·gc.alloc.rate.norm CopyOnWriteArrayList 10 >> 88.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm CopyOnWriteArrayList 100 >> 448.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm CopyOnWriteArrayList 1000 >> 4048.1 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm CopyOnWriteArrayList 10 >> 456.0 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm CopyOnWriteArrayList 100 >> 22057.2 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm CopyOnWriteArrayList 1000 >> 2020150.3 ± 7.3 B/op >> >> addAll:·gc.alloc.rate.norm ConcurrentLinkedDeque 10 >> 312.0 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm ConcurrentLinkedDeque 100 >> 2472.1 ± 0.0 B/op >> addAll:·gc.alloc.rate.norm ConcurrentLinkedDeque 1000 >> 24073.7 ± 0.1 B/op >> collectionsAddAll:·gc.alloc.rate.norm ConcurrentLinkedDeque 10 >> 288.0 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ConcurrentLinkedDeque 100 >> 2448.3 ± 0.0 B/op >> collectionsAddAll:·gc.alloc.rate.norm ConcurrentLinkedDeque 1000 >> 24051.4 ± 0.3 B/op >> There's never a case when `Collections.addAll` is fater - on the contrary >> `c.addAll(Arrays.asList())` always wins. Pay attention especially to >> dramatic difference for array-based collection. >> >> So I propose to reimplement the method by simply delegating to >> `Arrays.asList` because the spec declares identical behaviour and to remove >> perfomance notation from JavaDoc. > > Looks like I've found the original ticket: > https://bugs.openjdk.java.net/browse/JDK-8193031 Apart from the @SuppressWarnings, this looks good to me. And i like the irony of this. ------------- PR: https://git.openjdk.java.net/jdk/pull/1764
