From dbb5b6cfdac55110bafc8c1d0fa6aa9fd30f5379 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Mon, 3 Nov 2025 16:41:35 +0000 Subject: [PATCH 1/8] 8371164 Optimizing ArrayList.addAll() --- .../share/classes/java/util/ArrayList.java | 18 +- .../share/classes/java/util/Collections.java | 12 ++ .../java/util/ArrayListBulkOpsBenchmark.java | 179 ++++++++++++++++++ 3 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java diff --git a/src/java.base/share/classes/java/util/ArrayList.java b/src/java.base/share/classes/java/util/ArrayList.java index c00b130a553a2..c3524b47d9427 100644 --- a/src/java.base/share/classes/java/util/ArrayList.java +++ b/src/java.base/share/classes/java/util/ArrayList.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -750,6 +750,22 @@ public void clear() { * @throws NullPointerException if the specified collection is null */ public boolean addAll(Collection c) { + if (c.getClass() == ArrayList.class) { + ArrayList src = (ArrayList) c; + Object[] a = src.elementData; + int numNew = src.size; + if (numNew == 0) + return false; + modCount++; + Object[] elementData; + final int s; + if (numNew > (elementData = this.elementData).length - (s = size)) + elementData = grow(s + numNew); + System.arraycopy(a, 0, elementData, s, numNew); + size = s + numNew; + return true; + } + Object[] a = c.toArray(); modCount++; int numNew = a.length; diff --git a/src/java.base/share/classes/java/util/Collections.java b/src/java.base/share/classes/java/util/Collections.java index c48dbd8cf6c13..135bb5cba0f52 100644 --- a/src/java.base/share/classes/java/util/Collections.java +++ b/src/java.base/share/classes/java/util/Collections.java @@ -5253,6 +5253,18 @@ public boolean removeIf(Predicate filter) { public int hashCode() { return Objects.hashCode(element); } + public Object[] toArray() { + return new Object[] {element}; + } + @SuppressWarnings("unchecked") + public T[] toArray(T[] a) { + if (a.length < 1) + a = (T[])java.lang.reflect.Array.newInstance(a.getClass().getComponentType(), 1); + a[0] = (T)element; + if (a.length > 1) + a[1] = null; + return a; + } } /** diff --git a/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java b/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java new file mode 100644 index 0000000000000..d65260810bdb8 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java @@ -0,0 +1,179 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.bench.java.util; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.Set; +import java.util.TreeSet; +import java.util.WeakHashMap; +import java.util.concurrent.TimeUnit; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Benchmark measuring ArrayList addAll() performance. + * + * Tests the performance of ArrayList.addAll() when copying from another ArrayList. + */ +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Benchmark) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 1, jvmArgs = { "-XX:+UseParallelGC", "-Xmx3g" }) +public class ArrayListBulkOpsBenchmark { + + @Param({ "false", "true" }) + private boolean enablePoisoning; + + private ArrayList arrayListSource0; + private ArrayList arrayListSource5; + private ArrayList arrayListSource75; + private LinkedList linkedListSource0; + private LinkedList linkedListSource5; + private LinkedList linkedListSource75; + private Set singletonSetSource; + private HashMap hashMapSource; + private TreeSet treeSetSource; + private WeakHashMap weakHashMapSource; + + @Benchmark + public ArrayList addAllArrayList0(Blackhole bh) { + ArrayList result = new ArrayList<>(0); + result.addAll(arrayListSource0); + bh.consume(result); + return result; + } + + @Benchmark + public ArrayList addAllArrayList5(Blackhole bh) { + ArrayList result = new ArrayList<>(5); + result.addAll(arrayListSource5); + bh.consume(result); + return result; + } + + @Benchmark + public ArrayList addAllArrayList75(Blackhole bh) { + ArrayList result = new ArrayList<>(75); + result.addAll(arrayListSource75); + bh.consume(result); + return result; + } + + @Benchmark + public ArrayList addAllLinkedList0(Blackhole bh) { + ArrayList result = new ArrayList<>(0); + result.addAll(linkedListSource0); + bh.consume(result); + return result; + } + + @Benchmark + public ArrayList addAllLinkedList5(Blackhole bh) { + ArrayList result = new ArrayList<>(5); + result.addAll(linkedListSource5); + bh.consume(result); + return result; + } + + @Benchmark + public ArrayList addAllLinkedList75(Blackhole bh) { + ArrayList result = new ArrayList<>(75); + result.addAll(linkedListSource75); + bh.consume(result); + return result; + } + + @Benchmark + public ArrayList addAllSingletonSet(Blackhole bh) { + ArrayList result = new ArrayList<>(1); + result.addAll(singletonSetSource); + bh.consume(result); + return result; + } + + @Setup(Level.Trial) + public void setup() { + // Create source collections of different sizes + arrayListSource0 = new ArrayList<>(); + arrayListSource5 = new ArrayList<>(); + arrayListSource75 = new ArrayList<>(); + linkedListSource0 = new LinkedList<>(); + linkedListSource5 = new LinkedList<>(); + linkedListSource75 = new LinkedList<>(); + + for (int i = 0; i < 5; i++) { + arrayListSource5.add("key" + i); + linkedListSource5.add("key" + i); + } + + for (int i = 0; i < 75; i++) { + arrayListSource75.add("key" + i); + linkedListSource75.add("key" + i); + } + + // SingletonSet always contains exactly one element + singletonSetSource = Collections.singleton("key0"); + + // Create poisoning collections + hashMapSource = new HashMap<>(); + treeSetSource = new TreeSet<>(); + weakHashMapSource = new WeakHashMap<>(); + for (int i = 0; i < 75; i++) { + hashMapSource.put("key" + i, "value" + i); + treeSetSource.add("key" + i); + weakHashMapSource.put("key" + i, "value" + i); + } + + if (enablePoisoning) { + poisonCallSites(); + } + } + + private void poisonCallSites() { + // Poison ArrayList.addAll() with different Collection types + for (int i = 0; i < 40000; i++) { + ArrayList temp = new ArrayList<>(); + temp.addAll(hashMapSource.entrySet()); + temp.addAll(treeSetSource); + temp.addAll(weakHashMapSource.keySet()); + } + } +} From 626dabbe22cd3e131e67c6c991930f6a5248060d Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Tue, 4 Nov 2025 15:38:01 +0000 Subject: [PATCH 2/8] Adding direct unit tests, minor revisions to optimizations --- .../share/classes/java/util/ArrayList.java | 2 +- .../share/classes/java/util/Collections.java | 2 + test/jdk/java/util/ArrayList/AddAll.java | 145 ++++++++++++++++- .../util/Collections/SingletonSetToArray.java | 109 +++++++++++++ .../java/util/ArrayListBulkOpsBenchmark.java | 147 ++++++------------ 5 files changed, 303 insertions(+), 102 deletions(-) create mode 100644 test/jdk/java/util/Collections/SingletonSetToArray.java diff --git a/src/java.base/share/classes/java/util/ArrayList.java b/src/java.base/share/classes/java/util/ArrayList.java index c3524b47d9427..bf173dc4f2d96 100644 --- a/src/java.base/share/classes/java/util/ArrayList.java +++ b/src/java.base/share/classes/java/util/ArrayList.java @@ -754,9 +754,9 @@ public boolean addAll(Collection c) { ArrayList src = (ArrayList) c; Object[] a = src.elementData; int numNew = src.size; + modCount++; if (numNew == 0) return false; - modCount++; Object[] elementData; final int s; if (numNew > (elementData = this.elementData).length - (s = size)) diff --git a/src/java.base/share/classes/java/util/Collections.java b/src/java.base/share/classes/java/util/Collections.java index 135bb5cba0f52..f3342e9877e98 100644 --- a/src/java.base/share/classes/java/util/Collections.java +++ b/src/java.base/share/classes/java/util/Collections.java @@ -5253,9 +5253,11 @@ public boolean removeIf(Predicate filter) { public int hashCode() { return Objects.hashCode(element); } + @Override public Object[] toArray() { return new Object[] {element}; } + @Override @SuppressWarnings("unchecked") public T[] toArray(T[] a) { if (a.length < 1) diff --git a/test/jdk/java/util/ArrayList/AddAll.java b/test/jdk/java/util/ArrayList/AddAll.java index a0a2be0b3fa36..2db1720bd9304 100644 --- a/test/jdk/java/util/ArrayList/AddAll.java +++ b/test/jdk/java/util/ArrayList/AddAll.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,12 +23,16 @@ /* * @test - * @bug 4715206 + * @bug 4715206 8371164 * @summary Ensure that addAll method can cope with underestimate by size(). + * Test ArrayList-to-ArrayList fast path optimization. * @author Josh Bloch */ import java.util.ArrayList; +import java.util.Collections; +import java.util.ConcurrentModificationException; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -37,6 +41,14 @@ public class AddAll { public static void main(String[] args) { + testWeakHashMapAddAll(); + testArrayListFastPath(); + testArrayListSubclassUsesSlowPath(); + testSingletonSetToArray(); + testModCountIncrement(); + } + + private static void testWeakHashMapAddAll() { for (int j = 0; j < 1; j++) { Map m = new WeakHashMap(100000); for (int i = 0; i < 100000; i++) @@ -85,4 +97,133 @@ public static void main(String[] args) { list.addAll(1, m.keySet()); } } + + private static void testArrayListFastPath() { + // Test ArrayList-to-ArrayList fast path + ArrayList source = new ArrayList<>(); + source.add("a"); + source.add("b"); + source.add("c"); + + ArrayList dest = new ArrayList<>(); + dest.add("x"); + boolean result = dest.addAll(source); + + if (!result) { + throw new RuntimeException("addAll should return true when adding non-empty collection"); + } + if (dest.size() != 4) { + throw new RuntimeException("Expected size 4, got " + dest.size()); + } + if (!dest.get(0).equals("x") || !dest.get(1).equals("a") || + !dest.get(2).equals("b") || !dest.get(3).equals("c")) { + throw new RuntimeException("Elements not added correctly, got: " + dest); + } + + // Test empty ArrayList-to-ArrayList + ArrayList emptySource = new ArrayList<>(); + ArrayList dest2 = new ArrayList<>(); + dest2.add("y"); + boolean result2 = dest2.addAll(emptySource); + + if (result2) { + throw new RuntimeException("addAll should return false when adding empty collection"); + } + if (dest2.size() != 1 || !dest2.get(0).equals("y")) { + throw new RuntimeException("Destination should be unchanged when adding empty collection, got: " + dest2); + } + + // Test ArrayList fast path with null elements + ArrayList sourceWithNull = new ArrayList<>(); + sourceWithNull.add(null); + sourceWithNull.add("test"); + + ArrayList dest3 = new ArrayList<>(); + dest3.addAll(sourceWithNull); + + if (dest3.size() != 2 || dest3.get(0) != null || !dest3.get(1).equals("test")) { + throw new RuntimeException("Fast path should preserve null elements, got: " + dest3); + } + } + + private static void testSingletonSetToArray() { + // Test SingletonSet toArray() optimization + var set = Collections.singleton("test"); + Object[] array = set.toArray(); + + if (array.length != 1 || !array[0].equals("test")) { + throw new RuntimeException("SingletonSet toArray() failed, expected [test], got: " + java.util.Arrays.toString(array)); + } + + // Test SingletonSet toArray(T[]) with exact size + String[] stringArray = new String[1]; + String[] result = set.toArray(stringArray); + + if (result != stringArray || result.length != 1 || !result[0].equals("test")) { + throw new RuntimeException("SingletonSet toArray(T[]) with exact size failed, expected same array with [test], got: " + java.util.Arrays.toString(result)); + } + + // Test SingletonSet toArray(T[]) with larger array + String[] largerArray = new String[3]; + String[] result2 = set.toArray(largerArray); + + if (result2 != largerArray || !result2[0].equals("test") || result2[1] != null) { + throw new RuntimeException("SingletonSet toArray(T[]) with larger array failed, expected [test, null, ...], got: " + java.util.Arrays.toString(result2)); + } + + // Test SingletonSet toArray(T[]) with smaller array + String[] smallerArray = new String[0]; + String[] result3 = set.toArray(smallerArray); + + if (result3 == smallerArray || result3.length != 1 || !result3[0].equals("test")) { + throw new RuntimeException("SingletonSet toArray(T[]) with smaller array failed, expected new array [test], got: " + java.util.Arrays.toString(result3)); + } + } + + private static void testArrayListSubclassUsesSlowPath() { + // ArrayList subclasses should NOT use the fast path + class ArrayListSubclass extends ArrayList {} + + ArrayListSubclass source = new ArrayListSubclass<>(); + source.add("test"); + + ArrayList dest = new ArrayList<>(); + boolean result = dest.addAll(source); + + if (!result || dest.size() != 1 || !dest.get(0).equals("test")) { + throw new RuntimeException("ArrayList subclass addAll failed"); + } + } + + private static void testModCountIncrement() { + // Test that modCount is incremented by checking iterator behavior + ArrayList source = new ArrayList<>(); + source.add("test"); + + ArrayList dest = new ArrayList<>(); + Iterator it = dest.iterator(); + + dest.addAll(source); // Should increment modCount + + try { + it.next(); // Should throw ConcurrentModificationException + throw new RuntimeException("Expected ConcurrentModificationException"); + } catch (ConcurrentModificationException e) { + // Expected - modCount was incremented + } + + // Test that modCount is incremented even when adding empty ArrayList + ArrayList emptySource = new ArrayList<>(); + ArrayList dest2 = new ArrayList<>(); + Iterator it2 = dest2.iterator(); + + dest2.addAll(emptySource); // Should increment modCount (even though returns false) + + try { + it2.next(); // Should throw ConcurrentModificationException + throw new RuntimeException("Expected ConcurrentModificationException for empty addAll"); + } catch (ConcurrentModificationException e) { + // Expected - modCount was incremented even for empty collection + } + } } diff --git a/test/jdk/java/util/Collections/SingletonSetToArray.java b/test/jdk/java/util/Collections/SingletonSetToArray.java new file mode 100644 index 0000000000000..54acda7438b9e --- /dev/null +++ b/test/jdk/java/util/Collections/SingletonSetToArray.java @@ -0,0 +1,109 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8371164 + * @summary Test Collections.SingletonSet toArray() optimizations + * @author John Engebretson + */ + +import java.util.Collections; +import java.util.Set; + +public class SingletonSetToArray { + public static void main(String[] args) { + testToArray(); + testToArrayWithExactSize(); + testToArrayWithLargerArray(); + testToArrayWithSmallerArray(); + testToArrayWithNullElement(); + } + + private static void testToArray() { + Set set = Collections.singleton("test"); + Object[] array = set.toArray(); + + if (array.length != 1 || !array[0].equals("test")) { + throw new RuntimeException("toArray() failed, expected [test], got: " + java.util.Arrays.toString(array)); + } + } + + private static void testToArrayWithExactSize() { + Set set = Collections.singleton("test"); + String[] stringArray = new String[1]; + String[] result = set.toArray(stringArray); + + if (result != stringArray || result.length != 1 || !result[0].equals("test")) { + throw new RuntimeException("toArray(T[]) with exact size failed, expected same array with [test], got: " + java.util.Arrays.toString(result)); + } + } + + private static void testToArrayWithLargerArray() { + Set set = Collections.singleton("test"); + String[] largerArray = new String[5]; + String[] result = set.toArray(largerArray); + + if (result != largerArray || !result[0].equals("test") || result[1] != null) { + throw new RuntimeException("toArray(T[]) with larger array failed, expected [test, null, ...], got: " + java.util.Arrays.toString(result)); + } + + // Verify remaining elements are unchanged (should remain null) + for (int i = 2; i < largerArray.length; i++) { + if (largerArray[i] != null) { + throw new RuntimeException("Array element " + i + " should remain null, got: " + largerArray[i]); + } + } + } + + private static void testToArrayWithSmallerArray() { + Set set = Collections.singleton("test"); + String[] smallerArray = new String[0]; + String[] result = set.toArray(smallerArray); + + if (result == smallerArray || result.length != 1 || !result[0].equals("test")) { + throw new RuntimeException("toArray(T[]) with smaller array failed, expected new array [test], got: " + java.util.Arrays.toString(result)); + } + + // Verify correct array type was created + if (!result.getClass().getComponentType().equals(String.class)) { + throw new RuntimeException("Wrong array type created, expected String[], got: " + result.getClass().getComponentType()); + } + } + + private static void testToArrayWithNullElement() { + Set set = Collections.singleton(null); + Object[] array = set.toArray(); + + if (array.length != 1 || array[0] != null) { + throw new RuntimeException("toArray() with null element failed, expected [null], got: " + java.util.Arrays.toString(array)); + } + + String[] stringArray = new String[1]; + String[] result = set.toArray(stringArray); + + if (result != stringArray || result.length != 1 || result[0] != null) { + throw new RuntimeException("toArray(T[]) with null element failed, expected [null], got: " + java.util.Arrays.toString(result)); + } + } +} diff --git a/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java b/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java index d65260810bdb8..62696df70b9cb 100644 --- a/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java +++ b/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; +import java.util.List; import java.util.Set; import java.util.TreeSet; import java.util.WeakHashMap; @@ -44,7 +45,7 @@ import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Warmup; -import org.openjdk.jmh.infra.Blackhole; + /** * Benchmark measuring ArrayList addAll() performance. @@ -58,122 +59,70 @@ @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) @Fork(value = 1, jvmArgs = { "-XX:+UseParallelGC", "-Xmx3g" }) public class ArrayListBulkOpsBenchmark { + @Param({"0", "1", "5", "75"}) + int size; - @Param({ "false", "true" }) - private boolean enablePoisoning; - - private ArrayList arrayListSource0; - private ArrayList arrayListSource5; - private ArrayList arrayListSource75; - private LinkedList linkedListSource0; - private LinkedList linkedListSource5; - private LinkedList linkedListSource75; - private Set singletonSetSource; - private HashMap hashMapSource; - private TreeSet treeSetSource; - private WeakHashMap weakHashMapSource; - - @Benchmark - public ArrayList addAllArrayList0(Blackhole bh) { - ArrayList result = new ArrayList<>(0); - result.addAll(arrayListSource0); - bh.consume(result); - return result; - } - - @Benchmark - public ArrayList addAllArrayList5(Blackhole bh) { - ArrayList result = new ArrayList<>(5); - result.addAll(arrayListSource5); - bh.consume(result); - return result; - } - - @Benchmark - public ArrayList addAllArrayList75(Blackhole bh) { - ArrayList result = new ArrayList<>(75); - result.addAll(arrayListSource75); - bh.consume(result); - return result; - } + @Param({"ArrayList", "LinkedList"}) + String type; - @Benchmark - public ArrayList addAllLinkedList0(Blackhole bh) { - ArrayList result = new ArrayList<>(0); - result.addAll(linkedListSource0); - bh.consume(result); - return result; - } + List source; - @Benchmark - public ArrayList addAllLinkedList5(Blackhole bh) { - ArrayList result = new ArrayList<>(5); - result.addAll(linkedListSource5); - bh.consume(result); - return result; - } - - @Benchmark - public ArrayList addAllLinkedList75(Blackhole bh) { - ArrayList result = new ArrayList<>(75); - result.addAll(linkedListSource75); - bh.consume(result); - return result; + @Setup(Level.Trial) + public void setup() { + switch (type) { + case "ArrayList" -> source = new ArrayList<>(size); + case "LinkedList" -> source = new LinkedList<>(); + } + for (int i = 0; i < size; i++) source.add("key" + i); } @Benchmark - public ArrayList addAllSingletonSet(Blackhole bh) { - ArrayList result = new ArrayList<>(1); - result.addAll(singletonSetSource); - bh.consume(result); + public ArrayList addAll() { + ArrayList result = new ArrayList<>(size); + result.addAll(source); return result; } - @Setup(Level.Trial) - public void setup() { - // Create source collections of different sizes - arrayListSource0 = new ArrayList<>(); - arrayListSource5 = new ArrayList<>(); - arrayListSource75 = new ArrayList<>(); - linkedListSource0 = new LinkedList<>(); - linkedListSource5 = new LinkedList<>(); - linkedListSource75 = new LinkedList<>(); - - for (int i = 0; i < 5; i++) { - arrayListSource5.add("key" + i); - linkedListSource5.add("key" + i); - } - - for (int i = 0; i < 75; i++) { - arrayListSource75.add("key" + i); - linkedListSource75.add("key" + i); - } - - // SingletonSet always contains exactly one element - singletonSetSource = Collections.singleton("key0"); - - // Create poisoning collections - hashMapSource = new HashMap<>(); - treeSetSource = new TreeSet<>(); - weakHashMapSource = new WeakHashMap<>(); + static void poisonCallSites() { + HashMap hashMapSource = new HashMap<>(); + TreeSet treeSetSource = new TreeSet<>(); + WeakHashMap weakHashMapSource = new WeakHashMap<>(); for (int i = 0; i < 75; i++) { hashMapSource.put("key" + i, "value" + i); treeSetSource.add("key" + i); weakHashMapSource.put("key" + i, "value" + i); } - - if (enablePoisoning) { - poisonCallSites(); - } - } - - private void poisonCallSites() { // Poison ArrayList.addAll() with different Collection types - for (int i = 0; i < 40000; i++) { + for (int i = 0; i < 40_000; i++) { ArrayList temp = new ArrayList<>(); temp.addAll(hashMapSource.entrySet()); temp.addAll(treeSetSource); temp.addAll(weakHashMapSource.keySet()); } } -} + + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + @State(Scope.Benchmark) + @Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) + @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) + @Fork(value = 1, jvmArgs = { "-XX:+UseParallelGC", "-Xmx3g" }) + public static class SingletonSet { + Set singletonSetSource = Collections.singleton("key"); + + @Param({ "false", "true" }) + private boolean poison; + + @Setup(Level.Trial) + public void setup() { + if (poison) poisonCallSites(); + } + + @Benchmark + public ArrayList addAllSingletonSet() { + ArrayList result = new ArrayList<>(1); + result.addAll(singletonSetSource); + return result; + } + } +} \ No newline at end of file From 7725441a0791c4cf6673367e57c908ed87fe2ff5 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Thu, 6 Nov 2025 20:05:19 +0000 Subject: [PATCH 3/8] Simplifying ArrayList.addAll() fastpath --- .../share/classes/java/util/ArrayList.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/java.base/share/classes/java/util/ArrayList.java b/src/java.base/share/classes/java/util/ArrayList.java index bf173dc4f2d96..53e818b99c51f 100644 --- a/src/java.base/share/classes/java/util/ArrayList.java +++ b/src/java.base/share/classes/java/util/ArrayList.java @@ -750,25 +750,17 @@ public void clear() { * @throws NullPointerException if the specified collection is null */ public boolean addAll(Collection c) { + Object[] a; + int numNew; if (c.getClass() == ArrayList.class) { ArrayList src = (ArrayList) c; - Object[] a = src.elementData; - int numNew = src.size; - modCount++; - if (numNew == 0) - return false; - Object[] elementData; - final int s; - if (numNew > (elementData = this.elementData).length - (s = size)) - elementData = grow(s + numNew); - System.arraycopy(a, 0, elementData, s, numNew); - size = s + numNew; - return true; + a = src.elementData; + numNew = src.size; + } else { + a = c.toArray(); + numNew = a.length; } - - Object[] a = c.toArray(); modCount++; - int numNew = a.length; if (numNew == 0) return false; Object[] elementData; From 6c333bdc6ebf2172123e3712fefcd66e6fd2f608 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Mon, 10 Nov 2025 16:06:12 +0000 Subject: [PATCH 4/8] Cleaning up tests per PR feedback --- test/jdk/java/util/ArrayList/AddAll.java | 145 +----------------- test/jdk/java/util/Collection/MOAT.java | 36 +++++ .../util/Collections/SingletonSetToArray.java | 109 ------------- 3 files changed, 38 insertions(+), 252 deletions(-) delete mode 100644 test/jdk/java/util/Collections/SingletonSetToArray.java diff --git a/test/jdk/java/util/ArrayList/AddAll.java b/test/jdk/java/util/ArrayList/AddAll.java index 2db1720bd9304..a0a2be0b3fa36 100644 --- a/test/jdk/java/util/ArrayList/AddAll.java +++ b/test/jdk/java/util/ArrayList/AddAll.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,16 +23,12 @@ /* * @test - * @bug 4715206 8371164 + * @bug 4715206 * @summary Ensure that addAll method can cope with underestimate by size(). - * Test ArrayList-to-ArrayList fast path optimization. * @author Josh Bloch */ import java.util.ArrayList; -import java.util.Collections; -import java.util.ConcurrentModificationException; -import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -41,14 +37,6 @@ public class AddAll { public static void main(String[] args) { - testWeakHashMapAddAll(); - testArrayListFastPath(); - testArrayListSubclassUsesSlowPath(); - testSingletonSetToArray(); - testModCountIncrement(); - } - - private static void testWeakHashMapAddAll() { for (int j = 0; j < 1; j++) { Map m = new WeakHashMap(100000); for (int i = 0; i < 100000; i++) @@ -97,133 +85,4 @@ private static void testWeakHashMapAddAll() { list.addAll(1, m.keySet()); } } - - private static void testArrayListFastPath() { - // Test ArrayList-to-ArrayList fast path - ArrayList source = new ArrayList<>(); - source.add("a"); - source.add("b"); - source.add("c"); - - ArrayList dest = new ArrayList<>(); - dest.add("x"); - boolean result = dest.addAll(source); - - if (!result) { - throw new RuntimeException("addAll should return true when adding non-empty collection"); - } - if (dest.size() != 4) { - throw new RuntimeException("Expected size 4, got " + dest.size()); - } - if (!dest.get(0).equals("x") || !dest.get(1).equals("a") || - !dest.get(2).equals("b") || !dest.get(3).equals("c")) { - throw new RuntimeException("Elements not added correctly, got: " + dest); - } - - // Test empty ArrayList-to-ArrayList - ArrayList emptySource = new ArrayList<>(); - ArrayList dest2 = new ArrayList<>(); - dest2.add("y"); - boolean result2 = dest2.addAll(emptySource); - - if (result2) { - throw new RuntimeException("addAll should return false when adding empty collection"); - } - if (dest2.size() != 1 || !dest2.get(0).equals("y")) { - throw new RuntimeException("Destination should be unchanged when adding empty collection, got: " + dest2); - } - - // Test ArrayList fast path with null elements - ArrayList sourceWithNull = new ArrayList<>(); - sourceWithNull.add(null); - sourceWithNull.add("test"); - - ArrayList dest3 = new ArrayList<>(); - dest3.addAll(sourceWithNull); - - if (dest3.size() != 2 || dest3.get(0) != null || !dest3.get(1).equals("test")) { - throw new RuntimeException("Fast path should preserve null elements, got: " + dest3); - } - } - - private static void testSingletonSetToArray() { - // Test SingletonSet toArray() optimization - var set = Collections.singleton("test"); - Object[] array = set.toArray(); - - if (array.length != 1 || !array[0].equals("test")) { - throw new RuntimeException("SingletonSet toArray() failed, expected [test], got: " + java.util.Arrays.toString(array)); - } - - // Test SingletonSet toArray(T[]) with exact size - String[] stringArray = new String[1]; - String[] result = set.toArray(stringArray); - - if (result != stringArray || result.length != 1 || !result[0].equals("test")) { - throw new RuntimeException("SingletonSet toArray(T[]) with exact size failed, expected same array with [test], got: " + java.util.Arrays.toString(result)); - } - - // Test SingletonSet toArray(T[]) with larger array - String[] largerArray = new String[3]; - String[] result2 = set.toArray(largerArray); - - if (result2 != largerArray || !result2[0].equals("test") || result2[1] != null) { - throw new RuntimeException("SingletonSet toArray(T[]) with larger array failed, expected [test, null, ...], got: " + java.util.Arrays.toString(result2)); - } - - // Test SingletonSet toArray(T[]) with smaller array - String[] smallerArray = new String[0]; - String[] result3 = set.toArray(smallerArray); - - if (result3 == smallerArray || result3.length != 1 || !result3[0].equals("test")) { - throw new RuntimeException("SingletonSet toArray(T[]) with smaller array failed, expected new array [test], got: " + java.util.Arrays.toString(result3)); - } - } - - private static void testArrayListSubclassUsesSlowPath() { - // ArrayList subclasses should NOT use the fast path - class ArrayListSubclass extends ArrayList {} - - ArrayListSubclass source = new ArrayListSubclass<>(); - source.add("test"); - - ArrayList dest = new ArrayList<>(); - boolean result = dest.addAll(source); - - if (!result || dest.size() != 1 || !dest.get(0).equals("test")) { - throw new RuntimeException("ArrayList subclass addAll failed"); - } - } - - private static void testModCountIncrement() { - // Test that modCount is incremented by checking iterator behavior - ArrayList source = new ArrayList<>(); - source.add("test"); - - ArrayList dest = new ArrayList<>(); - Iterator it = dest.iterator(); - - dest.addAll(source); // Should increment modCount - - try { - it.next(); // Should throw ConcurrentModificationException - throw new RuntimeException("Expected ConcurrentModificationException"); - } catch (ConcurrentModificationException e) { - // Expected - modCount was incremented - } - - // Test that modCount is incremented even when adding empty ArrayList - ArrayList emptySource = new ArrayList<>(); - ArrayList dest2 = new ArrayList<>(); - Iterator it2 = dest2.iterator(); - - dest2.addAll(emptySource); // Should increment modCount (even though returns false) - - try { - it2.next(); // Should throw ConcurrentModificationException - throw new RuntimeException("Expected ConcurrentModificationException for empty addAll"); - } catch (ConcurrentModificationException e) { - // Expected - modCount was incremented even for empty collection - } - } } diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index ab9e4b4309d0e..09735983c84d3 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -887,6 +887,40 @@ private static void testStringElement(Collection c) { catch (Throwable t) { unexpected(t); } } + private static void testAddAll(Collection c) { + if (! supportsAdd(c)) return; + + clear(c); + + // Test empty ArrayList source + ArrayList emptySource = new ArrayList<>(); + check(! c.addAll(emptySource)); + + // Test non-empty ArrayList source + ArrayList arraySource = new ArrayList<>(); + arraySource.add(42); + arraySource.add(99); + check(c.addAll(arraySource)); + equal(new ArrayList(c), arraySource); + + clear(c); + + // Test non-ArrayList source + List linkedSource = new LinkedList<>(); + linkedSource.add(77); + check(c.addAll(linkedSource)); + equal(new ArrayList(c), linkedSource); + + // Test non-empty destination + clear(c); + c.add(10); + c.add(20); + int sizeBefore = c.size(); + check(c.addAll(arraySource)); + equal(c.size(), sizeBefore + arraySource.size()); + check(c.containsAll(arraySource)); + } + private static void testConcurrentCollection(Collection c) { try { c.add(1); @@ -1294,6 +1328,8 @@ private static void testCollection1(Collection c) { clear(c); testStringElement(c); oneElement(c); testStringElement(c); + testAddAll(c); + if (c.getClass().getName().matches(".*concurrent.*")) testConcurrentCollection(c); diff --git a/test/jdk/java/util/Collections/SingletonSetToArray.java b/test/jdk/java/util/Collections/SingletonSetToArray.java deleted file mode 100644 index 54acda7438b9e..0000000000000 --- a/test/jdk/java/util/Collections/SingletonSetToArray.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/* - * @test - * @bug 8371164 - * @summary Test Collections.SingletonSet toArray() optimizations - * @author John Engebretson - */ - -import java.util.Collections; -import java.util.Set; - -public class SingletonSetToArray { - public static void main(String[] args) { - testToArray(); - testToArrayWithExactSize(); - testToArrayWithLargerArray(); - testToArrayWithSmallerArray(); - testToArrayWithNullElement(); - } - - private static void testToArray() { - Set set = Collections.singleton("test"); - Object[] array = set.toArray(); - - if (array.length != 1 || !array[0].equals("test")) { - throw new RuntimeException("toArray() failed, expected [test], got: " + java.util.Arrays.toString(array)); - } - } - - private static void testToArrayWithExactSize() { - Set set = Collections.singleton("test"); - String[] stringArray = new String[1]; - String[] result = set.toArray(stringArray); - - if (result != stringArray || result.length != 1 || !result[0].equals("test")) { - throw new RuntimeException("toArray(T[]) with exact size failed, expected same array with [test], got: " + java.util.Arrays.toString(result)); - } - } - - private static void testToArrayWithLargerArray() { - Set set = Collections.singleton("test"); - String[] largerArray = new String[5]; - String[] result = set.toArray(largerArray); - - if (result != largerArray || !result[0].equals("test") || result[1] != null) { - throw new RuntimeException("toArray(T[]) with larger array failed, expected [test, null, ...], got: " + java.util.Arrays.toString(result)); - } - - // Verify remaining elements are unchanged (should remain null) - for (int i = 2; i < largerArray.length; i++) { - if (largerArray[i] != null) { - throw new RuntimeException("Array element " + i + " should remain null, got: " + largerArray[i]); - } - } - } - - private static void testToArrayWithSmallerArray() { - Set set = Collections.singleton("test"); - String[] smallerArray = new String[0]; - String[] result = set.toArray(smallerArray); - - if (result == smallerArray || result.length != 1 || !result[0].equals("test")) { - throw new RuntimeException("toArray(T[]) with smaller array failed, expected new array [test], got: " + java.util.Arrays.toString(result)); - } - - // Verify correct array type was created - if (!result.getClass().getComponentType().equals(String.class)) { - throw new RuntimeException("Wrong array type created, expected String[], got: " + result.getClass().getComponentType()); - } - } - - private static void testToArrayWithNullElement() { - Set set = Collections.singleton(null); - Object[] array = set.toArray(); - - if (array.length != 1 || array[0] != null) { - throw new RuntimeException("toArray() with null element failed, expected [null], got: " + java.util.Arrays.toString(array)); - } - - String[] stringArray = new String[1]; - String[] result = set.toArray(stringArray); - - if (result != stringArray || result.length != 1 || result[0] != null) { - throw new RuntimeException("toArray(T[]) with null element failed, expected [null], got: " + java.util.Arrays.toString(result)); - } - } -} From dd713ae010bb16ba2c819b62db0aa945ca30ecd0 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Mon, 10 Nov 2025 16:25:50 +0000 Subject: [PATCH 5/8] Whitespace fixes --- test/jdk/java/util/Collection/MOAT.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index 09735983c84d3..a87aabbef38f2 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -888,29 +888,30 @@ private static void testStringElement(Collection c) { } private static void testAddAll(Collection c) { - if (! supportsAdd(c)) return; - + if (!supportsAdd(c)) + return; + clear(c); - + // Test empty ArrayList source ArrayList emptySource = new ArrayList<>(); - check(! c.addAll(emptySource)); - + check(!c.addAll(emptySource)); + // Test non-empty ArrayList source ArrayList arraySource = new ArrayList<>(); arraySource.add(42); arraySource.add(99); check(c.addAll(arraySource)); equal(new ArrayList(c), arraySource); - + clear(c); - + // Test non-ArrayList source List linkedSource = new LinkedList<>(); linkedSource.add(77); check(c.addAll(linkedSource)); equal(new ArrayList(c), linkedSource); - + // Test non-empty destination clear(c); c.add(10); From 49b43473d76e9134927a63cefabb8b8b7b6d53af Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Thu, 13 Nov 2025 14:59:37 +0000 Subject: [PATCH 6/8] Unit test revisions --- test/jdk/java/util/Collection/MOAT.java | 39 +++++++++---------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index a87aabbef38f2..3c996c5d9d971 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -888,38 +888,25 @@ private static void testStringElement(Collection c) { } private static void testAddAll(Collection c) { - if (!supportsAdd(c)) - return; - clear(c); - // Test empty ArrayList source - ArrayList emptySource = new ArrayList<>(); - check(!c.addAll(emptySource)); - - // Test non-empty ArrayList source - ArrayList arraySource = new ArrayList<>(); - arraySource.add(42); - arraySource.add(99); - check(c.addAll(arraySource)); - equal(new ArrayList(c), arraySource); + // Test ArrayList source + ArrayList arrayListSource = new ArrayList<>(); + arrayListSource.add(42); + arrayListSource.add(99); + check(c.addAll(arrayListSource)); + equal(c.size(), arrayListSource.size()); + check(c.containsAll(arrayListSource)); clear(c); // Test non-ArrayList source - List linkedSource = new LinkedList<>(); - linkedSource.add(77); - check(c.addAll(linkedSource)); - equal(new ArrayList(c), linkedSource); - - // Test non-empty destination - clear(c); - c.add(10); - c.add(20); - int sizeBefore = c.size(); - check(c.addAll(arraySource)); - equal(c.size(), sizeBefore + arraySource.size()); - check(c.containsAll(arraySource)); + LinkedList linkedListSource = new LinkedList<>(); + linkedListSource.add(77); + linkedListSource.add(88); + check(c.addAll(linkedListSource)); + equal(c.size(), linkedListSource.size()); + check(c.containsAll(linkedListSource)); } private static void testConcurrentCollection(Collection c) { From 1fc9e9e000f4adef490eccd10521bcc0e00fe451 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Fri, 14 Nov 2025 15:51:20 +0000 Subject: [PATCH 7/8] Polishing test and benchmark --- test/jdk/java/util/Collection/MOAT.java | 2 +- .../org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index 3c996c5d9d971..4e1cb6e94814f 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -26,7 +26,7 @@ * @bug 6207984 6272521 6192552 6269713 6197726 6260652 5073546 4137464 * 4155650 4216399 4294891 6282555 6318622 6355327 6383475 6420753 * 6431845 4802633 6570566 6570575 6570631 6570924 6691185 6691215 - * 4802647 7123424 8024709 8193128 8327858 8368178 + * 4802647 7123424 8024709 8193128 8327858 8368178 8371164 * @summary Run many tests on many Collection and Map implementations * @author Martin Buchholz * @modules java.base/java.util:open diff --git a/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java b/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java index 62696df70b9cb..78b902f0c3add 100644 --- a/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java +++ b/test/micro/org/openjdk/bench/java/util/ArrayListBulkOpsBenchmark.java @@ -125,4 +125,4 @@ public ArrayList addAllSingletonSet() { return result; } } -} \ No newline at end of file +} From 04d9f35eac998c8d158df585557730d289be13ce Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Mon, 17 Nov 2025 15:37:30 +0000 Subject: [PATCH 8/8] Removing explicit classname --- src/java.base/share/classes/java/util/Collections.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/java/util/Collections.java b/src/java.base/share/classes/java/util/Collections.java index f3342e9877e98..316458d6f9091 100644 --- a/src/java.base/share/classes/java/util/Collections.java +++ b/src/java.base/share/classes/java/util/Collections.java @@ -5261,7 +5261,7 @@ public Object[] toArray() { @SuppressWarnings("unchecked") public T[] toArray(T[] a) { if (a.length < 1) - a = (T[])java.lang.reflect.Array.newInstance(a.getClass().getComponentType(), 1); + a = (T[])Array.newInstance(a.getClass().getComponentType(), 1); a[0] = (T)element; if (a.length > 1) a[1] = null;