RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations

[jengebr]

On Mon, 3 Jun 2024 17:07:28 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless cloning of Object[0] instances. This cloning is intended to prevent callers from changing array contents, but many `CopyOnWriteArrayList`s are allocated to size zero, or are otherwise maintained empty, so cloning is unnecessary.
>> 
>> Results from the included JMH benchmark:
>> Before:
>> 
>> Benchmark                                                    Mode  Cnt   Score   Error  Units
>> CopyOnWriteArrayListBenchmark.clear                          avgt    5  74.487 ± 1.793  ns/op
>> CopyOnWriteArrayListBenchmark.clearEmpty                     avgt    5  27.918 ± 0.759  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceArray            avgt    5  16.656 ± 0.375  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty       avgt    5  15.415 ± 0.489  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceCollection       avgt    5  21.608 ± 0.363  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty  avgt    5  15.374 ± 0.260  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceDefault          avgt    5  15.688 ± 0.350  ns/op
>> 
>> 
>> After:
>> 
>> Benchmark                                                    Mode  Cnt   Score   Error  Units
>> CopyOnWriteArrayListBenchmark.clear                          avgt    5  75.365 ± 2.092  ns/op
>> CopyOnWriteArrayListBenchmark.clearEmpty                     avgt    5  20.803 ± 0.539  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceArray            avgt    5  16.808 ± 0.582  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty       avgt    5  12.980 ± 0.418  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceCollection       avgt    5  21.627 ± 0.173  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty  avgt    5  12.864 ± 0.408  ns/op
>> CopyOnWriteArrayListBenchmark.createInstanceDefault          avgt    5  12.931 ± 0.255  ns/op
>
> src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java line 338:
> 
>> 336:      */
>> 337:     public Object[] toArray() {
>> 338:         return getArray().length == 0 ? getArray() : getArray().clone();
> 
> Unfortunately, I think this is against the spec for this method, which explicitly says the method must allocate a new array. Yes, this change would be within the spirit of the spec ("You can modify the array", which you cannot if the object is empty), but is against the letter of it.

I think you're right, I'll remove the change to toArray().

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19527#discussion_r1624873832