From 2622772b751979386ed4e45bcf31e7f831d6cae2 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Tue, 11 Nov 2025 18:22:13 +0000 Subject: [PATCH 1/4] Optimizing HashMap.putAll() and . for HashMap and C$UM --- .../share/classes/java/util/Collections.java | 2 +- .../share/classes/java/util/HashMap.java | 29 +++- test/jdk/java/util/Collection/MOAT.java | 39 +++++ .../util/HashMapConstructorBenchmark.java | 162 ++++++++++++++++++ 4 files changed, 227 insertions(+), 5 deletions(-) create mode 100644 test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java diff --git a/src/java.base/share/classes/java/util/Collections.java b/src/java.base/share/classes/java/util/Collections.java index c48dbd8cf6c13..78a12ad1d8dc5 100644 --- a/src/java.base/share/classes/java/util/Collections.java +++ b/src/java.base/share/classes/java/util/Collections.java @@ -1642,7 +1642,7 @@ public static Map unmodifiableMap(Map m) { /** * @serial include */ - private static class UnmodifiableMap implements Map, Serializable { + static class UnmodifiableMap implements Map, Serializable { @java.io.Serial private static final long serialVersionUID = -1034234728574286014L; diff --git a/src/java.base/share/classes/java/util/HashMap.java b/src/java.base/share/classes/java/util/HashMap.java index 3320b394e6c4c..7b71066449f43 100644 --- a/src/java.base/share/classes/java/util/HashMap.java +++ b/src/java.base/share/classes/java/util/HashMap.java @@ -493,6 +493,16 @@ public HashMap(Map m) { putMapEntries(m, false); } + @SuppressWarnings({"unchecked"}) + private void putMapEntries(HashMap src, boolean evict) { + for (Node node : src.table) { + while (node != null) { + putVal(node.hash, node.key, node.value, false, evict); + node = node.next; + } + } + } + /** * Implements Map.putAll and Map constructor. * @@ -501,6 +511,11 @@ public HashMap(Map m) { * true (relayed to method afterNodeInsertion). */ final void putMapEntries(Map m, boolean evict) { + if (m.getClass() == Collections.UnmodifiableMap.class) { + @SuppressWarnings("unchecked") + Map unwrapped = ((Collections.UnmodifiableMap) m).m; + m = unwrapped; + } int s = m.size(); if (s > 0) { if (table == null) { // pre-size @@ -517,10 +532,16 @@ final void putMapEntries(Map m, boolean evict) { resize(); } - for (Map.Entry e : m.entrySet()) { - K key = e.getKey(); - V value = e.getValue(); - putVal(hash(key), key, value, false, evict); + if (m.getClass() == HashMap.class) { + @SuppressWarnings("unchecked") + HashMap hashMap = (HashMap) m; + putMapEntries(hashMap, evict); + } else { + for (Map.Entry e : m.entrySet()) { + K key = e.getKey(); + V value = e.getValue(); + putVal(hash(key), key, value, false, evict); + } } } } diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index ab9e4b4309d0e..7c5f59849ccb9 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -143,6 +143,7 @@ public static void realMain(String[] args) { testImmutableSet(AccessFlag.maskToAccessFlags(Modifier.PUBLIC | Modifier.STATIC | Modifier.SYNCHRONIZED, AccessFlag.Location.METHOD), AccessFlag.ABSTRACT); testImmutableList(unmodifiableList(Arrays.asList(1,2,3))); testImmutableMap(unmodifiableMap(Collections.singletonMap(1,2))); + testImmutableMap(unmodifiableMap(new HashMap<>(Map.of(1, 101, 2, 202, 3, 303)))); testImmutableSeqColl(unmodifiableSequencedCollection(Arrays.asList(1,2,3)), 99); testImmutableSeqColl(unmodifiableSequencedSet(new LinkedHashSet<>(Arrays.asList(1,2,3))), 99); var lhm = new LinkedHashMap(); lhm.put(1,2); lhm.put(3, 4); @@ -157,6 +158,8 @@ public static void realMain(String[] args) { testMapMutatorsAlwaysThrow(unmodifiableMap(Collections.emptyMap())); testEmptyMapMutatorsAlwaysThrow(unmodifiableMap(Collections.emptyMap())); + testHashMapPutAll(); + // Empty collections final List emptyArray = Arrays.asList(new Integer[]{}); testCollection(emptyArray); @@ -419,6 +422,27 @@ public static void realMain(String[] args) { testMapMutatorsAlwaysThrow(mapCollected2); } + // Test HashMap.putAll() with various source map types + private static void testHashMapPutAll() { + Map testData = Map.of(1, 101, 2, 202, 3, 303); + HashMap target = new HashMap<>(); + + target.putAll(new TreeMap<>(testData)); + check(target.equals(testData)); + + target.clear(); + target.putAll(new ConcurrentHashMap<>(testData)); + check(target.equals(testData)); + + target.clear(); + target.putAll(unmodifiableMap(new HashMap<>(testData))); + check(target.equals(testData)); + + target.clear(); + target.putAll(unmodifiableMap(new TreeMap<>(testData))); + check(target.equals(testData)); + } + private static void checkContainsSelf(Collection c) { check(c.containsAll(c)); check(c.containsAll(Arrays.asList(c.toArray()))); @@ -715,6 +739,11 @@ private static void testImmutableMap(final Map m) { () -> m.remove(first), () -> m.clear()); testImmutableMapEntry(m.entrySet().iterator().next()); + + // Test putAll from immutable map to HashMap + HashMap target = new HashMap<>(); + target.putAll(m); + check(target.equals(m)); } testImmutableSet(m.keySet(), 99); testImmutableCollection(m.values(), 99); @@ -1423,6 +1452,16 @@ private static void testMap(Map m) { check(m.size() == 2); checkFunctionalInvariants(m); checkNPEConsistency(m); + + // Test putAll with HashMap + HashMap source = new HashMap<>(); + source.put(1, 101); + source.put(2, 202); + source.put(3, 303); + + m.clear(); + m.putAll(source); + check(m.equals(source)); } catch (Throwable t) { unexpected(t); } } diff --git a/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java b/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java new file mode 100644 index 0000000000000..c462d466f2fb8 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java @@ -0,0 +1,162 @@ +package org.openjdk.bench.java.util; + +import org.openjdk.jmh.annotations.*; + +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; + +/** + * Benchmark demonstrating performance impact of polymorphic call sites on HashMap.(Map). + * + * This test shows that manual inlining of HashMap construction can significantly outperform + * the built-in HashMap(Map) constructor when the constructor call site becomes polymorphic. + * + * The setup ensures polymorphic call sites by using HashMap, TreeMap, and LinkedHashMap + * in both the constructor and manual iteration patterns before benchmarking begins. + */ +@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 HashMapConstructorBenchmark { + + private static final int POISON_ITERATIONS = 40000; + private static final double CAPACITY_FACTOR = 1.35; // Account for 0.75 load factor + + @Param({"0", "5", "25"}) + private int mapSize; + + @Param({"true", "false"}) + private boolean poisonCallSites; + + @Param({"HashMap", "TreeMap", "ConcurrentHashMap", "UnmodifiableMap(HashMap)", "UnmodifiableMap(TreeMap)"}) + private String inputType; + + private HashMap inputHashMap; + private TreeMap inputTreeMap; + private LinkedHashMap inputLinkedHashMap; + private ConcurrentHashMap inputConcurrentHashMap; + private WeakHashMap inputWeakHashMap; + private Map inputSynchronizedMap; + private Map inputUnmodifiableMap; + private Map inputUnmodifiableTreeMap; + + private Map sourceMap; + + @Setup(Level.Trial) + public void setup() { + // Create test data with identical contents + inputHashMap = new HashMap<>(); + inputTreeMap = new TreeMap<>(); + inputLinkedHashMap = new LinkedHashMap<>(); + inputConcurrentHashMap = new ConcurrentHashMap<>(); + inputWeakHashMap = new WeakHashMap<>(); + + for (int i = 0; i < mapSize; i++) { + String key = "key" + i; + Integer value = i; + inputHashMap.put(key, value); + inputTreeMap.put(key, value); + inputLinkedHashMap.put(key, value); + inputConcurrentHashMap.put(key, value); + inputWeakHashMap.put(key, value); + } + + // Create wrapper maps for poisoning + inputSynchronizedMap = Collections.synchronizedMap(new HashMap<>(inputHashMap)); + inputUnmodifiableMap = Collections.unmodifiableMap(new HashMap<>(inputHashMap)); + inputUnmodifiableTreeMap = Collections.unmodifiableMap(new TreeMap<>(inputTreeMap)); + + // Set source map based on inputType parameter + sourceMap = switch (inputType) { + case "HashMap" -> inputHashMap; + case "TreeMap" -> inputTreeMap; + case "ConcurrentHashMap" -> inputConcurrentHashMap; + case "UnmodifiableMap(HashMap)" -> inputUnmodifiableMap; + case "UnmodifiableMap(TreeMap)" -> inputUnmodifiableTreeMap; + default -> throw new IllegalArgumentException("Unknown inputType: " + inputType); + }; + + if (poisonCallSites) { + poisonCallSites(); + } + } + + private void poisonCallSites() { + @SuppressWarnings("unchecked") + Map[] sources = new Map[] { inputHashMap, inputTreeMap, inputLinkedHashMap, + inputConcurrentHashMap, inputWeakHashMap }; + + // Poison HashMap.(Map) call site + for (int i = 0; i < POISON_ITERATIONS; i++) { + Map source = sources[i % sources.length]; + HashMap temp = new HashMap<>(source); + if (temp.size() != mapSize) + throw new RuntimeException(); + } + + // Poison entrySet iteration call sites + for (int i = 0; i < POISON_ITERATIONS; i++) { + Map source = sources[i % sources.length]; + HashMap temp = new HashMap<>(source.size()); + for (Map.Entry entry : source.entrySet()) { + temp.put(entry.getKey(), entry.getValue()); + } + if (temp.size() != mapSize) + throw new RuntimeException(); + } + + // Poison UnmodifiableMap call sites + @SuppressWarnings("unchecked") + Map[] umSources = new Map[]{ + Collections.unmodifiableMap(inputHashMap), + Collections.unmodifiableMap(inputTreeMap), + Collections.unmodifiableMap(inputLinkedHashMap), + Collections.unmodifiableMap(inputConcurrentHashMap), + Collections.unmodifiableMap(inputWeakHashMap) + }; + + for (int i = 0; i < POISON_ITERATIONS; i++) { + Map source = umSources[i % umSources.length]; + HashMap temp = new HashMap<>(source); + if (temp.size() != mapSize) + throw new RuntimeException(); + } + + for (int i = 0; i < POISON_ITERATIONS; i++) { + Map source = umSources[i % umSources.length]; + HashMap temp = new HashMap<>(source.size()); + for (Map.Entry entry : source.entrySet()) { + temp.put(entry.getKey(), entry.getValue()); + } + if (temp.size() != mapSize) throw new RuntimeException(); + } + } + + /** + * Benchmark using HashMap's built-in constructor that takes a Map parameter. + * This approach suffers from polymorphic call site overhead. + */ + @Benchmark + public HashMap hashMapConstructor() { + return new HashMap<>(sourceMap); + } + + /** + * Benchmark using manual iteration over entrySet with individual put() calls. + * This approach bypasses bulk operations and their polymorphic call sites. + */ + /* + @Benchmark + public HashMap manualEntrySetLoop() { + HashMap result = new HashMap<>(); + for (Map.Entry entry : sourceMap.entrySet()) { + result.put(entry.getKey(), entry.getValue()); + } + return result; + } + */ +} From ac9cf4badf41e0ba36b9ed99bafcfb626f27e141 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Tue, 11 Nov 2025 19:44:02 +0000 Subject: [PATCH 2/4] fixing whitespace --- .../bench/java/util/HashMapConstructorBenchmark.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java b/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java index c462d466f2fb8..320f5e405ae1b 100644 --- a/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java +++ b/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java @@ -8,10 +8,10 @@ /** * Benchmark demonstrating performance impact of polymorphic call sites on HashMap.(Map). - * + * * This test shows that manual inlining of HashMap construction can significantly outperform * the built-in HashMap(Map) constructor when the constructor call site becomes polymorphic. - * + * * The setup ensures polymorphic call sites by using HashMap, TreeMap, and LinkedHashMap * in both the constructor and manual iteration patterns before benchmarking begins. */ @@ -113,7 +113,7 @@ private void poisonCallSites() { @SuppressWarnings("unchecked") Map[] umSources = new Map[]{ Collections.unmodifiableMap(inputHashMap), - Collections.unmodifiableMap(inputTreeMap), + Collections.unmodifiableMap(inputTreeMap), Collections.unmodifiableMap(inputLinkedHashMap), Collections.unmodifiableMap(inputConcurrentHashMap), Collections.unmodifiableMap(inputWeakHashMap) From 05891484a8b8f2c9e162507fd7c5a631bb6cb355 Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Tue, 11 Nov 2025 21:30:33 +0000 Subject: [PATCH 3/4] Bug fix & unit test --- src/java.base/share/classes/java/util/HashMap.java | 10 ++++++---- test/jdk/java/util/Collection/MOAT.java | 6 ++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/java.base/share/classes/java/util/HashMap.java b/src/java.base/share/classes/java/util/HashMap.java index 7b71066449f43..ac0d4916df3b8 100644 --- a/src/java.base/share/classes/java/util/HashMap.java +++ b/src/java.base/share/classes/java/util/HashMap.java @@ -495,10 +495,12 @@ public HashMap(Map m) { @SuppressWarnings({"unchecked"}) private void putMapEntries(HashMap src, boolean evict) { - for (Node node : src.table) { - while (node != null) { - putVal(node.hash, node.key, node.value, false, evict); - node = node.next; + if (src.table != null) { + for (Node node : src.table) { + while (node != null) { + putVal(node.hash, node.key, node.value, false, evict); + node = node.next; + } } } } diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index 7c5f59849ccb9..3528518ca5c35 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -441,6 +441,12 @@ private static void testHashMapPutAll() { target.clear(); target.putAll(unmodifiableMap(new TreeMap<>(testData))); check(target.equals(testData)); + + // Test empty HashMap putAll (regression test for NPE) + target.clear(); + HashMap emptySource = new HashMap<>(); + target.putAll(emptySource); + check(target.isEmpty()); } private static void checkContainsSelf(Collection c) { From edfc6a2f5ecc3a92e569603045320f84ec1b01ab Mon Sep 17 00:00:00 2001 From: John Engebretson Date: Thu, 13 Nov 2025 15:14:18 +0000 Subject: [PATCH 4/4] Unit test revisions --- test/jdk/java/util/Collection/MOAT.java | 19 ++++++++++--------- .../util/HashMapConstructorBenchmark.java | 17 ++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/test/jdk/java/util/Collection/MOAT.java b/test/jdk/java/util/Collection/MOAT.java index 3528518ca5c35..6f4f0b11be59b 100644 --- a/test/jdk/java/util/Collection/MOAT.java +++ b/test/jdk/java/util/Collection/MOAT.java @@ -422,31 +422,32 @@ public static void realMain(String[] args) { testMapMutatorsAlwaysThrow(mapCollected2); } - // Test HashMap.putAll() with various source map types + // Test HashMap.putAll() optimization paths private static void testHashMapPutAll() { Map testData = Map.of(1, 101, 2, 202, 3, 303); HashMap target = new HashMap<>(); - target.putAll(new TreeMap<>(testData)); + target.putAll(new HashMap<>(testData)); + equal(target.size(), testData.size()); check(target.equals(testData)); target.clear(); - target.putAll(new ConcurrentHashMap<>(testData)); + + target.putAll(new TreeMap<>(testData)); + equal(target.size(), testData.size()); check(target.equals(testData)); target.clear(); + target.putAll(unmodifiableMap(new HashMap<>(testData))); + equal(target.size(), testData.size()); check(target.equals(testData)); target.clear(); + target.putAll(unmodifiableMap(new TreeMap<>(testData))); + equal(target.size(), testData.size()); check(target.equals(testData)); - - // Test empty HashMap putAll (regression test for NPE) - target.clear(); - HashMap emptySource = new HashMap<>(); - target.putAll(emptySource); - check(target.isEmpty()); } private static void checkContainsSelf(Collection c) { diff --git a/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java b/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java index 320f5e405ae1b..c72693fee4550 100644 --- a/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java +++ b/test/micro/org/openjdk/bench/java/util/HashMapConstructorBenchmark.java @@ -7,13 +7,13 @@ import java.util.concurrent.TimeUnit; /** - * Benchmark demonstrating performance impact of polymorphic call sites on HashMap.(Map). + * Benchmark comparing HashMap constructor performance against manual iteration. * - * This test shows that manual inlining of HashMap construction can significantly outperform - * the built-in HashMap(Map) constructor when the constructor call site becomes polymorphic. + * Tests HashMap.(Map) performance across different source map types, with and without + * call site poisoning to simulate real-world megamorphic conditions. * - * The setup ensures polymorphic call sites by using HashMap, TreeMap, and LinkedHashMap - * in both the constructor and manual iteration patterns before benchmarking begins. + * The setup poisons polymorphic call sites by using five different map types + * in both the constructor and manual iteration patterns to ensure megamorphic behavior. */ @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(TimeUnit.NANOSECONDS) @@ -24,7 +24,6 @@ public class HashMapConstructorBenchmark { private static final int POISON_ITERATIONS = 40000; - private static final double CAPACITY_FACTOR = 1.35; // Account for 0.75 load factor @Param({"0", "5", "25"}) private int mapSize; @@ -40,7 +39,6 @@ public class HashMapConstructorBenchmark { private LinkedHashMap inputLinkedHashMap; private ConcurrentHashMap inputConcurrentHashMap; private WeakHashMap inputWeakHashMap; - private Map inputSynchronizedMap; private Map inputUnmodifiableMap; private Map inputUnmodifiableTreeMap; @@ -66,7 +64,6 @@ public void setup() { } // Create wrapper maps for poisoning - inputSynchronizedMap = Collections.synchronizedMap(new HashMap<>(inputHashMap)); inputUnmodifiableMap = Collections.unmodifiableMap(new HashMap<>(inputHashMap)); inputUnmodifiableTreeMap = Collections.unmodifiableMap(new TreeMap<>(inputTreeMap)); @@ -138,7 +135,7 @@ private void poisonCallSites() { /** * Benchmark using HashMap's built-in constructor that takes a Map parameter. - * This approach suffers from polymorphic call site overhead. + * Performance varies based on source map type and call site polymorphism. */ @Benchmark public HashMap hashMapConstructor() { @@ -149,7 +146,6 @@ public HashMap hashMapConstructor() { * Benchmark using manual iteration over entrySet with individual put() calls. * This approach bypasses bulk operations and their polymorphic call sites. */ - /* @Benchmark public HashMap manualEntrySetLoop() { HashMap result = new HashMap<>(); @@ -158,5 +154,4 @@ public HashMap manualEntrySetLoop() { } return result; } - */ }