8210280: Unnecessary reallocation when invoking HashMap.putAll()
Reviewed-by: martin, mvala, igerasim, chegar, rriggs
Contributed-by: Michal Vala <mvala@redhat.com>, Doug Lea <dl@cs.oswego.edu>, Martin Buchholz <martinrb@google.com>
--- a/src/java.base/share/classes/java/util/HashMap.java Fri Feb 08 14:03:09 2019 -0500
+++ b/src/java.base/share/classes/java/util/HashMap.java Fri Feb 08 13:39:22 2019 -0800
@@ -501,9 +501,14 @@
(int)ft : MAXIMUM_CAPACITY);
if (t > threshold)
threshold = tableSizeFor(t);
+ } else {
+ // Because of linked-list bucket constraints, we cannot
+ // expand all at once, but can reduce total resize
+ // effort by repeated doubling now vs later
+ while (s > threshold && table.length < MAXIMUM_CAPACITY)
+ resize();
}
- else if (s > threshold)
- resize();
+
for (Map.Entry<? extends K, ? extends V> e : m.entrySet()) {
K key = e.getKey();
V value = e.getValue();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/HashMap/WhiteBoxResizeTest.java Fri Feb 08 13:39:22 2019 -0800
@@ -0,0 +1,132 @@
+/*
+ * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ *
+ * 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.
+ */
+
+import org.testng.annotations.Test;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.invoke.VarHandle;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Supplier;
+import java.util.stream.IntStream;
+
+import static java.util.stream.Collectors.toMap;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
+
+/*
+ * @test
+ * @bug 8210280
+ * @modules java.base/java.util:open
+ * @summary White box tests for HashMap internals around table resize
+ * @run testng WhiteBoxResizeTest
+ * @key randomness
+ */
+public class WhiteBoxResizeTest {
+ final ThreadLocalRandom rnd = ThreadLocalRandom.current();
+ final MethodHandle TABLE_SIZE_FOR;
+ final VarHandle THRESHOLD;
+ final VarHandle TABLE;
+
+ public WhiteBoxResizeTest() throws ReflectiveOperationException {
+ Class<?> mClass = HashMap.class;
+ String nodeClassName = mClass.getName() + "$Node";
+ Class<?> nodeArrayClass = Class.forName("[L" + nodeClassName + ";");
+ MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(mClass, MethodHandles.lookup());
+ TABLE = lookup.findVarHandle(mClass, "table", nodeArrayClass);
+ this.TABLE_SIZE_FOR = lookup.findStatic(
+ mClass, "tableSizeFor",
+ MethodType.methodType(int.class, int.class));
+ this.THRESHOLD = lookup.findVarHandle(mClass, "threshold", int.class);
+ }
+
+ int tableSizeFor(int n) {
+ try {
+ return (int) TABLE_SIZE_FOR.invoke(n);
+ } catch (Throwable t) { throw new AssertionError(t); }
+ }
+
+ Object[] table(HashMap map) {
+ try {
+ return (Object[]) TABLE.get(map);
+ } catch (Throwable t) { throw new AssertionError(t); }
+ }
+
+ int capacity(HashMap map) {
+ return table(map).length;
+ }
+
+ @Test
+ public void testTableSizeFor() {
+ assertEquals(tableSizeFor(0), 1);
+ assertEquals(tableSizeFor(1), 1);
+ assertEquals(tableSizeFor(2), 2);
+ assertEquals(tableSizeFor(3), 4);
+ assertEquals(tableSizeFor(15), 16);
+ assertEquals(tableSizeFor(16), 16);
+ assertEquals(tableSizeFor(17), 32);
+ int maxSize = 1 << 30;
+ assertEquals(tableSizeFor(maxSize - 1), maxSize);
+ assertEquals(tableSizeFor(maxSize), maxSize);
+ assertEquals(tableSizeFor(maxSize + 1), maxSize);
+ assertEquals(tableSizeFor(Integer.MAX_VALUE), maxSize);
+ }
+
+ @Test
+ public void capacityTestDefaultConstructor() {
+ capacityTestDefaultConstructor(new HashMap<>());
+ capacityTestDefaultConstructor(new LinkedHashMap<>());
+ }
+
+ void capacityTestDefaultConstructor(HashMap<Integer, Integer> map) {
+ assertNull(table(map));
+
+ map.put(1, 1);
+ assertEquals(capacity(map), 16); // default initial capacity
+
+ map.putAll(IntStream.range(0, 64).boxed().collect(toMap(i -> i, i -> i)));
+ assertEquals(capacity(map), 128);
+ }
+
+ @Test
+ public void capacityTestInitialCapacity() {
+ int initialCapacity = rnd.nextInt(2, 128);
+ List<Supplier<HashMap<Integer, Integer>>> suppliers = List.of(
+ () -> new HashMap<>(initialCapacity),
+ () -> new HashMap<>(initialCapacity, 0.75f),
+ () -> new LinkedHashMap<>(initialCapacity),
+ () -> new LinkedHashMap<>(initialCapacity, 0.75f));
+
+ for (Supplier<HashMap<Integer, Integer>> supplier : suppliers) {
+ HashMap<Integer, Integer> map = supplier.get();
+ assertNull(table(map));
+
+ map.put(1, 1);
+ assertEquals(capacity(map), tableSizeFor(initialCapacity));
+ }
+ }
+}
--- a/test/jdk/java/util/concurrent/tck/JSR166TestCase.java Fri Feb 08 14:03:09 2019 -0500
+++ b/test/jdk/java/util/concurrent/tck/JSR166TestCase.java Fri Feb 08 13:39:22 2019 -0800
@@ -487,6 +487,12 @@
public static boolean atLeastJava9() { return JAVA_CLASS_VERSION >= 53.0; }
public static boolean atLeastJava10() { return JAVA_CLASS_VERSION >= 54.0; }
public static boolean atLeastJava11() { return JAVA_CLASS_VERSION >= 55.0; }
+ public static boolean atLeastJava12() { return JAVA_CLASS_VERSION >= 56.0; }
+ public static boolean atLeastJava13() { return JAVA_CLASS_VERSION >= 57.0; }
+ public static boolean atLeastJava14() { return JAVA_CLASS_VERSION >= 58.0; }
+ public static boolean atLeastJava15() { return JAVA_CLASS_VERSION >= 59.0; }
+ public static boolean atLeastJava16() { return JAVA_CLASS_VERSION >= 60.0; }
+ public static boolean atLeastJava17() { return JAVA_CLASS_VERSION >= 61.0; }
/**
* Collects all JSR166 unit tests as one suite.
@@ -577,6 +583,7 @@
"HashMapTest",
"LinkedBlockingDeque8Test",
"LinkedBlockingQueue8Test",
+ "LinkedHashMapTest",
"LongAccumulatorTest",
"LongAdderTest",
"SplittableRandomTest",
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/concurrent/tck/LinkedHashMapTest.java Fri Feb 08 13:39:22 2019 -0800
@@ -0,0 +1,60 @@
+/*
+ * 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.
+ */
+
+/*
+ * This file is available under and governed by the GNU General Public
+ * License version 2 only, as published by the Free Software Foundation.
+ * However, the following notice accompanied the original version of this
+ * file:
+ *
+ * Written by Doug Lea and Martin Buchholz with assistance from
+ * members of JCP JSR-166 Expert Group and released to the public
+ * domain, as explained at
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import junit.framework.Test;
+
+public class LinkedHashMapTest extends JSR166TestCase {
+ public static void main(String[] args) {
+ main(suite(), args);
+ }
+
+ public static Test suite() {
+ class Implementation implements MapImplementation {
+ public Class<?> klazz() { return LinkedHashMap.class; }
+ public Map emptyMap() { return new LinkedHashMap(); }
+ public Object makeKey(int i) { return i; }
+ public Object makeValue(int i) { return i; }
+ public boolean isConcurrent() { return false; }
+ public boolean permitsNullKeys() { return true; }
+ public boolean permitsNullValues() { return true; }
+ public boolean supportsSetValue() { return true; }
+ }
+ return newTestSuite(
+ // LinkedHashMapTest.class,
+ MapTest.testSuite(new Implementation()));
+ }
+}
--- a/test/jdk/java/util/concurrent/tck/MapTest.java Fri Feb 08 14:03:09 2019 -0500
+++ b/test/jdk/java/util/concurrent/tck/MapTest.java Fri Feb 08 13:39:22 2019 -0800
@@ -166,6 +166,40 @@
assertEquals(1, m.size());
}
+ /**
+ * "Missing" test found while investigating JDK-8210280.
+ * ant -Djsr166.tckTestClass=HashMapTest -Djsr166.methodFilter=testBug8210280 -Djsr166.runsPerTest=1000000 tck
+ */
+ public void testBug8210280() {
+ final ThreadLocalRandom rnd = ThreadLocalRandom.current();
+ final int size1 = rnd.nextInt(32);
+ final int size2 = rnd.nextInt(128);
+
+ final Map m1 = impl.emptyMap();
+ for (int i = 0; i < size1; i++) {
+ int elt = rnd.nextInt(1024 * i, 1024 * (i + 1));
+ assertNull(m1.put(impl.makeKey(elt), impl.makeValue(elt)));
+ }
+
+ final Map m2 = impl.emptyMap();
+ for (int i = 0; i < size2; i++) {
+ int elt = rnd.nextInt(Integer.MIN_VALUE + 1024 * i,
+ Integer.MIN_VALUE + 1024 * (i + 1));
+ assertNull(m2.put(impl.makeKey(elt), impl.makeValue(-elt)));
+ }
+
+ final Map m1Copy = impl.emptyMap();
+ m1Copy.putAll(m1);
+
+ m1.putAll(m2);
+
+ for (Object elt : m2.keySet())
+ assertEquals(m2.get(elt), m1.get(elt));
+ for (Object elt : m1Copy.keySet())
+ assertSame(m1Copy.get(elt), m1.get(elt));
+ assertEquals(size1 + size2, m1.size());
+ }
+
// public void testFailsIntentionallyForDebugging() {
// fail(impl.klazz().getSimpleName());
// }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/micro/org/openjdk/bench/java/util/HashMapBench.java Fri Feb 08 13:39:22 2019 -0800
@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ *
+ * 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 org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+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 java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+import java.util.stream.IntStream;
+
+import static java.util.stream.Collectors.toMap;
+
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@State(Scope.Thread)
+public class HashMapBench {
+ private Supplier<Map<Integer, Integer>> mapSupplier;
+ private Map<Integer, Integer> bigMapToAdd;
+
+ @Param("1000000")
+ private int size;
+
+ @Param
+ private MapType mapType;
+
+ public enum MapType {
+ HASH_MAP,
+ LINKED_HASH_MAP,
+ }
+
+ @Setup
+ public void setup() {
+ switch (mapType) {
+ case HASH_MAP:
+ mapSupplier = () -> new HashMap<>();
+ break;
+ case LINKED_HASH_MAP:
+ mapSupplier = () -> new LinkedHashMap<>();
+ break;
+ default:
+ throw new AssertionError();
+ }
+
+ ThreadLocalRandom rnd = ThreadLocalRandom.current();
+ this.bigMapToAdd = IntStream.range(0, size).boxed()
+ .collect(toMap(i -> 7 + i * 128, i -> rnd.nextInt()));
+ }
+
+ @Benchmark
+ public int putAllWithBigMapToNonEmptyMap() {
+ Map<Integer, Integer> map = mapSupplier.get();
+ map.put(-1, -1);
+ map.putAll(bigMapToAdd);
+ return map.size();
+ }
+
+ @Benchmark
+ public int putAllWithBigMapToEmptyMap() {
+ Map<Integer, Integer> map = mapSupplier.get();
+ map.putAll(bigMapToAdd);
+ return map.size();
+ }
+
+ @Benchmark
+ public int put() {
+ Map<Integer, Integer> map = mapSupplier.get();
+ for (int k : bigMapToAdd.keySet()) {
+ map.put(k, bigMapToAdd.get(k));
+ }
+ return map.size();
+ }
+}