Skip to content

Commit 089216a

Browse files
jderegclaude
andcommitted
Add hash spreading to MultiKeyMap for better bucket distribution
Apply ConcurrentHashMap-style spread (h ^ (h >>> 16)) at all 7 bucket selection points to mix higher hash bits into lower bits, reducing collisions when the table is small. Stored hashes are unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6dd6d79 commit 089216a

3 files changed

Lines changed: 49 additions & 43 deletions

File tree

changelog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
* `getMultiKey()` and `containsMultiKey()` methods never nulled out array entries after use
4343
* In thread-pool environments, this pinned references to user objects for the lifetime of the thread
4444
* Added `try/finally` cleanup in all 8 methods
45+
* **PERFORMANCE**: `MultiKeyMap` - Hash spreading for better bucket distribution
46+
* Added `spread(h) = h ^ (h >>> 16)` at all bucket selection points (same technique as `ConcurrentHashMap`)
47+
* When the table is small, only low-order bits select the bucket; spreading mixes in higher bits to reduce collisions
48+
* Applied at all 7 bucket index computation sites; stored hashes are unchanged
4549

4650
#### 4.90.0 2026-02-02
4751
* **BUG FIX**: `DeepEquals` - URL comparison now uses string representation instead of `URL.equals()`

src/main/java/com/cedarsoftware/util/MultiKeyMap.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -969,12 +969,22 @@ private static int hashDouble(double d) {
969969
return (int) (bits ^ (bits >>> 32));
970970
}
971971

972+
/**
973+
* Spreads higher bits of the hash into lower bits to improve bucket distribution.
974+
* When the table is small, only the low-order bits select the bucket. Without
975+
* spreading, hashes that differ only in higher bits collide in the same bucket.
976+
* This is the same technique used by {@code ConcurrentHashMap}.
977+
*/
978+
private static int spread(int h) {
979+
return h ^ (h >>> 16);
980+
}
981+
972982
private int getStripeIndex(int hash) {
973983
// Use bucket index for stripe selection to reduce false contention
974984
// between independent buckets that happen to have same low-order hash bits
975985
final AtomicReferenceArray<MultiKey<V>[]> table = buckets; // Volatile read of buckets
976986
final int mask = table.length() - 1; // Array length is immutable
977-
return (hash & mask) & STRIPE_MASK;
987+
return (spread(hash) & mask) & STRIPE_MASK;
978988
}
979989

980990
private ReentrantLock getStripeLock(int hash) {
@@ -1165,7 +1175,7 @@ private MultiKey<V> findSimpleOrComplexKey(Object key) {
11651175
int hash = computeElementHash(key, caseSensitive);
11661176
final AtomicReferenceArray<MultiKey<V>[]> table = buckets;
11671177
final int mask = table.length() - 1;
1168-
final int index = hash & mask;
1178+
final int index = spread(hash) & mask;
11691179
final MultiKey<V>[] chain = table.get(index);
11701180
if (chain != null) {
11711181
// Fast scan for single-key entries only
@@ -2072,7 +2082,7 @@ private static int expandAndHash(Object current, List<Object> result, IdentitySe
20722082
private MultiKey<V> findEntryWithPrecomputedHash(final Object normalizedKey, final int hash) {
20732083
final AtomicReferenceArray<MultiKey<V>[]> table = buckets; // Volatile read of buckets
20742084
final int mask = table.length() - 1; // Array length is immutable
2075-
final int index = hash & mask;
2085+
final int index = spread(hash) & mask;
20762086

20772087
final MultiKey<V>[] chain = table.get(index);
20782088
if (chain == null) {
@@ -3132,7 +3142,7 @@ private V getNoLock(MultiKey<V> lookupKey) {
31323142
int hash = lookupKey.hash;
31333143
final AtomicReferenceArray<MultiKey<V>[]> table = buckets; // Volatile read of buckets
31343144
final int mask = table.length() - 1; // Array length is immutable
3135-
int index = hash & mask;
3145+
int index = spread(hash) & mask;
31363146
MultiKey<V>[] chain = table.get(index);
31373147

31383148
if (chain == null) return null;
@@ -3150,7 +3160,7 @@ private V putNoLock(MultiKey<V> newKey) {
31503160
int hash = newKey.hash;
31513161
final AtomicReferenceArray<MultiKey<V>[]> table = buckets; // Volatile read of buckets
31523162
final int mask = table.length() - 1; // Array length is immutable
3153-
int index = hash & mask;
3163+
int index = spread(hash) & mask;
31543164
MultiKey<V>[] chain = table.get(index);
31553165

31563166
if (chain == null) {
@@ -3371,7 +3381,7 @@ private V removeNoLock(MultiKey<V> removeKey) {
33713381
int hash = removeKey.hash;
33723382
final AtomicReferenceArray<MultiKey<V>[]> table = buckets; // Volatile read of buckets
33733383
final int mask = table.length() - 1; // Array length is immutable
3374-
int index = hash & mask;
3384+
int index = spread(hash) & mask;
33753385
MultiKey<V>[] chain = table.get(index);
33763386

33773387
if (chain == null) return null;
@@ -3426,7 +3436,7 @@ private void resizeInternal() {
34263436

34273437
@SuppressWarnings("unchecked")
34283438
private int rehashEntry(MultiKey<V> entry, AtomicReferenceArray<MultiKey<V>[]> target) {
3429-
int index = entry.hash & (target.length() - 1);
3439+
int index = spread(entry.hash) & (target.length() - 1);
34303440
MultiKey<V>[] chain = target.get(index);
34313441
if (chain == null) {
34323442
target.set(index, new MultiKey[]{entry});

src/test/java/com/cedarsoftware/util/MultiKeyMapStripeTrackingTest.java

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,19 @@
1313
/**
1414
* Test for bug #5: Stripe contention diagnostics track the wrong stripe.
1515
*
16-
* Bug: putInternal and removeInternal compute the stripe index for contention
16+
* Bug: putInternal and removeInternal computed the stripe index for contention
1717
* tracking as {@code hash & STRIPE_MASK}, but getStripeLock computes it as
18-
* {@code (hash & tableMask) & STRIPE_MASK}. When the table is smaller than
19-
* STRIPE_COUNT (e.g., default capacity 16 vs STRIPE_COUNT 32), these produce
20-
* different values. For example, with hash=112, tableMask=15, STRIPE_MASK=31:
21-
* - getStripeLock: (112 & 15) & 31 = 0 (actual lock acquired on stripe 0)
22-
* - putInternal: 112 & 31 = 16 (contention tracked on stripe 16)
23-
*
24-
* This means per-stripe contention metrics are attributed to the wrong stripes.
18+
* {@code (spread(hash) & tableMask) & STRIPE_MASK}. When the table is smaller
19+
* than STRIPE_COUNT, these produce different values, so per-stripe metrics
20+
* were attributed to incorrect stripes.
2521
*/
2622
class MultiKeyMapStripeTrackingTest {
2723

24+
/** Mirrors MultiKeyMap.spread() */
25+
private static int spread(int h) {
26+
return h ^ (h >>> 16);
27+
}
28+
2829
@Test
2930
void testPutTracksAcquisitionOnCorrectStripe() throws Exception {
3031
MultiKeyMap<String> map = new MultiKeyMap<>(16);
@@ -48,36 +49,31 @@ void testPutTracksAcquisitionOnCorrectStripe() throws Exception {
4849
return; // Can't trigger on this machine/config
4950
}
5051

51-
// Find a key whose hash has bits set in (stripeMask & ~tableMask),
52-
// causing the buggy and correct stripe computations to differ.
53-
int diffBits = stripeMask & ~tableMask;
54-
52+
// Find a key where the correct stripe (with spread) differs from
53+
// the old buggy stripe (hash & STRIPE_MASK without spread or tableMask)
5554
String testKey = null;
5655
int testHash = 0;
5756
for (int i = 0; i < 10000; i++) {
5857
String candidate = "key" + i;
5958
int h = candidate.hashCode();
60-
if ((h & diffBits) != 0) {
59+
int correctStripe = (spread(h) & tableMask) & stripeMask;
60+
int buggyStripe = h & stripeMask;
61+
if (correctStripe != buggyStripe) {
6162
testKey = candidate;
6263
testHash = h;
6364
break;
6465
}
6566
}
66-
assertNotNull(testKey, "Should find a key with hash bits in the differing range");
67+
assertNotNull(testKey, "Should find a key where correct and buggy stripes differ");
6768

68-
int correctStripe = (testHash & tableMask) & stripeMask;
69-
int buggyStripe = testHash & stripeMask;
70-
assertNotEquals(correctStripe, buggyStripe,
71-
"Precondition: correct and buggy stripes must differ for this key");
69+
int expectedStripe = (spread(testHash) & tableMask) & stripeMask;
7270

7371
// Perform a put
7472
map.put(testKey, "value");
7573

76-
// Verify acquisition was tracked on the correct stripe (the one matching getStripeLock)
77-
assertEquals(1, acq[correctStripe].get(),
78-
"Acquisition should be tracked on stripe " + correctStripe + " (matching getStripeLock)");
79-
assertEquals(0, acq[buggyStripe].get(),
80-
"Stripe " + buggyStripe + " should have no acquisitions (wrong stripe)");
74+
// Verify acquisition was tracked on the correct stripe
75+
assertEquals(1, acq[expectedStripe].get(),
76+
"Acquisition should be tracked on stripe " + expectedStripe + " (matching getStripeIndex)");
8177
}
8278

8379
@Test
@@ -101,36 +97,32 @@ void testRemoveTracksAcquisitionOnCorrectStripe() throws Exception {
10197
return;
10298
}
10399

104-
int diffBits = stripeMask & ~tableMask;
105-
106100
String testKey = null;
107101
int testHash = 0;
108102
for (int i = 0; i < 10000; i++) {
109103
String candidate = "key" + i;
110104
int h = candidate.hashCode();
111-
if ((h & diffBits) != 0) {
105+
int correctStripe = (spread(h) & tableMask) & stripeMask;
106+
int buggyStripe = h & stripeMask;
107+
if (correctStripe != buggyStripe) {
112108
testKey = candidate;
113109
testHash = h;
114110
break;
115111
}
116112
}
117113
assertNotNull(testKey);
118114

119-
int correctStripe = (testHash & tableMask) & stripeMask;
120-
int buggyStripe = testHash & stripeMask;
121-
assertNotEquals(correctStripe, buggyStripe);
115+
int expectedStripe = (spread(testHash) & tableMask) & stripeMask;
122116

123117
// Put the key first (this also increments the correct stripe's counter)
124118
map.put(testKey, "value");
125-
int acqAfterPut = acq[correctStripe].get();
119+
int acqAfterPut = acq[expectedStripe].get();
126120

127121
// Now remove it - should also track on the correct stripe
128122
map.remove(testKey);
129123

130-
assertEquals(acqAfterPut + 1, acq[correctStripe].get(),
131-
"Remove acquisition should be tracked on stripe " + correctStripe);
132-
assertEquals(0, acq[buggyStripe].get(),
133-
"Stripe " + buggyStripe + " should have no acquisitions");
124+
assertEquals(acqAfterPut + 1, acq[expectedStripe].get(),
125+
"Remove acquisition should be tracked on stripe " + expectedStripe);
134126
}
135127

136128
@Test
@@ -155,8 +147,8 @@ void testNoAcquisitionsAboveTableSizeStripes() throws Exception {
155147
map.put("item" + i, "val" + i);
156148
}
157149

158-
// With table size 16 and stripe count 32, the correct stripe index
159-
// is (hash & 15) & 31 which can only produce values in [0, 15].
150+
// With table size 16 and stripe count 32, the stripe index is
151+
// (spread(hash) & 15) & 31 which can only produce values in [0, 15].
160152
// Stripes [16, 31] should never receive acquisitions.
161153
for (int s = tableSize; s < acq.length; s++) {
162154
assertEquals(0, acq[s].get(),

0 commit comments

Comments
 (0)