On Mon, 3 Jun 2024 17:07:28 GMT, Aleksey Shipilev <[email protected]> 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
