On Mon, 14 Dec 2020 12:13: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

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

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

Reply via email to