RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations

[Aleksey Shipilev]

On Mon, 3 Jun 2024 16:47:20 GMT, jengebr <duke 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

Some initial comments. @DougLea might want to give it a sanity check.

Note there is a jcheck failure due to whitespace issues. Plus, I think the PR should still be named "8332842: Optimize empty CopyOnWriteArrayList allocations"?

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.

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

PR Review: https://git.openjdk.org/jdk/pull/19527#pullrequestreview-2094435376
PR Comment: https://git.openjdk.org/jdk/pull/19527#issuecomment-2145853813
PR Review Comment: https://git.openjdk.org/jdk/pull/19527#discussion_r1624791561