Addressing review feedback on benchmark & test

This commit is contained in:
John Engebretson 2026-04-21 13:20:58 +00:00
parent e7ba79e65f
commit 792f28b65a
2 changed files with 30 additions and 69 deletions

View File

@ -428,25 +428,21 @@ public class MOAT {
HashMap<Integer,Integer> target = new HashMap<>();
target.putAll(new HashMap<>(testData));
equal(target.size(), testData.size());
check(target.equals(testData));
target.clear();
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));
}
@ -746,11 +742,6 @@ public class MOAT {
() -> m.remove(first),
() -> m.clear());
testImmutableMapEntry(m.entrySet().iterator().next());
// Test putAll from immutable map to HashMap
HashMap<Integer,Integer> target = new HashMap<>();
target.putAll(m);
check(target.equals(m));
}
testImmutableSet(m.keySet(), 99);
testImmutableCollection(m.values(), 99);
@ -1475,24 +1466,11 @@ public class MOAT {
if (supportsPut(m)) {
try {
check(m.put(3333, 77777) == null);
check(m.put(9134, 74982) == null);
check(m.get(9134) == 74982);
check(m.put(9134, 1382) == 74982);
check(m.get(9134) == 1382);
check(m.size() == 2);
checkFunctionalInvariants(m);
checkNPEConsistency(m);
// Test putAll with HashMap
HashMap<Integer,Integer> source = new HashMap<>();
source.put(1, 101);
source.put(2, 202);
source.put(3, 303);
m.clear();
int oldSize = m.size();
Map<Integer,Integer> source = Map.of(10, 1000, 11, 1001, 12, 1002);
m.putAll(source);
check(m.equals(source));
check(m.entrySet().containsAll(source.entrySet()));
check(m.size() == oldSize + source.size());
}
catch (Throwable t) { unexpected(t); }
}

View File

@ -23,6 +23,7 @@
package org.openjdk.bench.java.util;
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.infra.Blackhole;
import java.math.BigInteger;
import java.util.*;
@ -30,7 +31,9 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
/**
* Benchmark comparing HashMap constructor performance against manual iteration.
* Benchmark comparing HashMap constructor performance against manual iteration - see
* JDK-8368292 for details of the targeted megamorphic issue and JDK-8371656 for details
* of the special-case optimization.
*
* Tests HashMap.<init>(Map) performance across different source map types, with and without
* call site poisoning to simulate real-world megamorphic conditions.
@ -38,8 +41,9 @@ import java.util.concurrent.TimeUnit;
* Uses BigInteger keys whose hashCode() is not cached and scales with magnitude,
* exposing the cost of hash recomputation in non-optimized paths.
*
* The setup poisons polymorphic call sites by using five different map types
* The setup poisons polymorphic call sites by using ten different map types
* in both the constructor and manual iteration patterns to ensure megamorphic behavior.
*
*/
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@ -49,7 +53,7 @@ import java.util.concurrent.TimeUnit;
@Fork(value = 1, jvmArgs = {"-XX:+UseParallelGC", "-Xmx3g"})
public class HashMapConstructorBenchmark {
private static final int POISON_ITERATIONS = 40000;
private static final int POISON_ITERATIONS = 80000;
@Param({"0", "5", "25", "150"})
private int mapSize;
@ -71,7 +75,7 @@ public class HashMapConstructorBenchmark {
private Map<BigInteger, Integer> sourceMap;
@Setup(Level.Trial)
public void setup() {
public void setup(Blackhole bh) {
Random rng = new Random(0);
inputHashMap = new HashMap<>();
@ -103,58 +107,37 @@ public class HashMapConstructorBenchmark {
};
if (poisonCallSites) {
poisonCallSites();
poisonCallSites(bh);
}
}
private void poisonCallSites() {
@SuppressWarnings("unchecked")
Map<BigInteger, Integer>[] sources = new Map[] { inputHashMap, inputTreeMap, inputLinkedHashMap,
inputConcurrentHashMap, inputWeakHashMap };
private void poisonCallSites(Blackhole bh) {
List<Map<BigInteger, Integer>> sources = List.of(inputHashMap,
inputTreeMap,
inputLinkedHashMap,
inputConcurrentHashMap,
inputWeakHashMap,
inputUnmodifiableMap,
inputUnmodifiableTreeMap,
Collections.unmodifiableMap(inputLinkedHashMap),
Collections.unmodifiableMap(inputConcurrentHashMap),
Collections.unmodifiableMap(inputWeakHashMap));
// Poison HashMap.<init>(Map) call site
for (int i = 0; i < POISON_ITERATIONS; i++) {
Map<BigInteger, Integer> source = sources[i % sources.length];
Map<BigInteger, Integer> source = sources.get(i % sources.size());
HashMap<BigInteger, Integer> temp = new HashMap<>(source);
if (temp.size() != mapSize)
throw new RuntimeException();
bh.consume(temp);
}
// Poison entrySet iteration call sites
for (int i = 0; i < POISON_ITERATIONS; i++) {
Map<BigInteger, Integer> source = sources[i % sources.length];
HashMap<BigInteger, Integer> temp = new HashMap<>(source.size());
Map<BigInteger, Integer> source = sources.get(i % sources.size());
HashMap<BigInteger, Integer> temp = HashMap.newHashMap(source.size());
for (Map.Entry<BigInteger, Integer> entry : source.entrySet()) {
temp.put(entry.getKey(), entry.getValue());
}
if (temp.size() != mapSize)
throw new RuntimeException();
}
// Poison UnmodifiableMap call sites
@SuppressWarnings("unchecked")
Map<BigInteger, Integer>[] 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<BigInteger, Integer> source = umSources[i % umSources.length];
HashMap<BigInteger, Integer> temp = new HashMap<>(source);
if (temp.size() != mapSize)
throw new RuntimeException();
}
for (int i = 0; i < POISON_ITERATIONS; i++) {
Map<BigInteger, Integer> source = umSources[i % umSources.length];
HashMap<BigInteger, Integer> temp = new HashMap<>(source.size());
for (Map.Entry<BigInteger, Integer> entry : source.entrySet()) {
temp.put(entry.getKey(), entry.getValue());
}
if (temp.size() != mapSize) throw new RuntimeException();
bh.consume(temp);
}
}
@ -173,7 +156,7 @@ public class HashMapConstructorBenchmark {
*/
@Benchmark
public HashMap<BigInteger, Integer> manualEntrySetLoop() {
HashMap<BigInteger, Integer> result = new HashMap<>();
HashMap<BigInteger, Integer> result = HashMap.newHashMap(sourceMap.size());
for (Map.Entry<BigInteger, Integer> entry : sourceMap.entrySet()) {
result.put(entry.getKey(), entry.getValue());
}