From 044c3dc6b6736dd07c641902abc003ae3a95d055 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Wed, 20 Nov 2024 12:56:46 +1100 Subject: [PATCH 1/2] Test how many times OpenHashMap hashes the key. 3 is too many times. In a future commit, we will reduce this. The goal here is to demonstrate that the future commit works. --- .../objects/Object2IntOpenHashMapTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java b/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java index 730d001b..c8bcba78 100644 --- a/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java +++ b/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java @@ -16,6 +16,7 @@ package it.unimi.dsi.fastutil.objects; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -215,5 +216,23 @@ public void test1000() throws IOException, ClassNotFoundException { public void testLegacyMainMethodTests() throws Exception { MainRunner.callMainIfExists(Object2IntOpenHashMap.class, "test", /*num=*/"500", /*loadFactor=*/"0.75", /*seed=*/"383454"); } + + /** Counts times hashCode() is called, so we can test double-hashing. */ + private static final class HashCounter { + int hashCount = 0; + @Override + public int hashCode() { + hashCount++; + return super.hashCode(); + } + } + + @Test + public void testMergeIntHashesKeyMultipleTimes() { + Object2IntOpenHashMap m = new Object2IntOpenHashMap(Hash.DEFAULT_INITIAL_SIZE); + HashCounter hc = new HashCounter(); + m.mergeInt(hc, 0, Math::max); + assertEquals(3, hc.hashCount); + } } From 87bb74c8b1e51d90395ff1a879926a11991df185 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Tue, 19 Nov 2024 10:16:01 +1100 Subject: [PATCH 2/2] OpenHashMap.mergeX: Avoid triple-hashing key Previously, we were hashing the key 3x: - get - containsKey - put Now, we override mergeDouble with a hashamp-specific implementation that hashes the key once. Towards #336 --- drv/OpenHashMap.drv | 19 +++++++++++++++++++ .../objects/Object2IntOpenHashMapTest.java | 5 +++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drv/OpenHashMap.drv b/drv/OpenHashMap.drv index 3d3de447..79cf189b 100644 --- a/drv/OpenHashMap.drv +++ b/drv/OpenHashMap.drv @@ -1253,6 +1253,25 @@ public class OPEN_HASH_MAP KEY_VALUE_GENERIC extends ABSTRACT_MAP KEY_VALUE_GENE return value[pos] = newVal; } +#if VALUES_PRIMITIVE && ! VALUE_CLASS_Boolean + /** {@inheritDoc} */ + @Override + public VALUE_GENERIC_TYPE MERGE_VALUE(final KEY_GENERIC_TYPE k, final VALUE_GENERIC_TYPE v, METHOD_ARG_VALUE_BINARY_OPERATOR remappingFunction) { + java.util.Objects.requireNonNull(remappingFunction); + REQUIRE_VALUE_NON_NULL(v) + + final int pos = find(k); + if (pos < 0) { + insert(-pos - 1, k, v); + return v; + } + + final VALUE_GENERIC_TYPE newValue = remappingFunction.VALUE_OPERATOR_APPLY(value[pos], v); + + return value[pos] = newValue; + } +#endif + /** {@inheritDoc} */ @Override public VALUE_GENERIC_TYPE merge(final KEY_GENERIC_TYPE k, final VALUE_GENERIC_TYPE v, final java.util.function.BiFunction remappingFunction) { diff --git a/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java b/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java index c8bcba78..674e028d 100644 --- a/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java +++ b/test/it/unimi/dsi/fastutil/objects/Object2IntOpenHashMapTest.java @@ -227,12 +227,13 @@ public int hashCode() { } } + // Regression test for https://github.com/vigna/fastutil/pull/337. @Test - public void testMergeIntHashesKeyMultipleTimes() { + public void testMergeIntHashesKeyOnce() { Object2IntOpenHashMap m = new Object2IntOpenHashMap(Hash.DEFAULT_INITIAL_SIZE); HashCounter hc = new HashCounter(); m.mergeInt(hc, 0, Math::max); - assertEquals(3, hc.hashCount); + assertEquals(1, hc.hashCount); } }