Skip to content

Commit c25dd99

Browse files
bobzhangclaude
andcommitted
perf(builtin): derive Map's probe-sequence length instead of storing it
`Entry` carried a `mut psl : Int` alongside `hash`, `key` and `value`. That field is not independent state: an entry's probe-sequence length is the distance between the slot it occupies and the slot its hash asked for, so psl = (idx - (hash & capacity_mask)) & capacity_mask Every write to it happened as the entry landed in a slot and always equalled that displacement, and every read happens inside a probe loop that already knows the slot index. `psl_at` computes it where it is needed and the field is gone, leaving five fields per entry instead of six. `shift_back` no longer decrements anything: moving an entry one slot back lowers its derived PSL by exactly one. `push_away` and `rehash_place_entry` likewise stop writing PSLs and just place entries. I checked the premise before relying on it rather than reasoning about it. Instrumenting `set_entry` and `add_entry_to_tail` -- every path that places an entry in a slot -- to assert `psl == (idx - (hash & mask)) & mask` on each write, the full suite passes on native, js and wasm-gc with that assertion live: 7544, 7485 and 7544 tests, no violation. No layout change: the table is still `FixedArray[Entry[K, V]?]`, so nothing here shares the tradeoffs of the struct-of-arrays experiments on #4127 and #4131. Measured against `main`, n=50000, interleaved in one session: | backend | op | main | this | change | | ------- | -- | ---- | ---- | ------ | | js | `set` | 3.04 ms | 2.90 ms | 5% faster | | js | `get` hit | 1.01 ms | 1.00 ms | unchanged | | js | `get` miss | 1.16 ms | 1.15 ms | unchanged | | js | `set+remove` | 4.30 ms | 4.29 ms | unchanged | | native | `set` | 2.62 ms | 2.63 ms | unchanged | | native | `get` hit | 855 us | 832 us | 3% faster | | native | `set+remove` | 3.82 ms | 3.73 ms | 2% faster | | wasm-gc | `set` | 2.46 ms | 2.42 ms | 2% faster | | wasm-gc | `get` hit | 1.07 ms | 1.07 ms | unchanged | Insertion gains where the smaller entry matters most and nothing regresses; the derivation costs two arithmetic operations per probe step against a field read from an object already in cache, which is why lookups come out flat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent aced026 commit c25dd99

2 files changed

Lines changed: 56 additions & 60 deletions

File tree

builtin/linked_hash_map.mbt

Lines changed: 52 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
priv struct Entry[K, V] {
2020
mut prev : Int
2121
mut next : Entry[K, V]?
22-
mut psl : Int
2322
hash : Int
2423
key : K
2524
mut value : V
@@ -74,6 +73,16 @@ struct Map[K, V] {
7473
// that does not depend on the list invariant. It carries that proof at its
7574
// definition, and it is what makes its `set_entry` call sound.
7675

76+
///|
77+
/// The probe-sequence length of the entry occupying slot `idx`: how far it
78+
/// sits from the slot its hash asked for. Derived rather than stored, since
79+
/// an entry's PSL is fully determined by where it ended up -- every write to
80+
/// it happened as the entry landed in a slot, always equal to this value.
81+
#inline
82+
fn[K, V] Map::psl_at(self : Map[K, V], idx : Int, entry : Entry[K, V]) -> Int {
83+
(idx - (entry.hash & self.capacity_mask)) & self.capacity_mask
84+
}
85+
7786
///|
7887
let default_init_capacity = 8
7988

@@ -173,7 +182,7 @@ fn[K : Eq, V] Map::set_with_hash(
173182
// Restart search with new capacity_mask
174183
continue 0, hash & self.capacity_mask
175184
}
176-
let entry = { prev: self.tail, next: None, psl, key, value, hash }
185+
let entry = { prev: self.tail, next: None, key, value, hash }
177186
self.add_entry_to_tail(idx, entry)
178187
return
179188
}
@@ -183,15 +192,15 @@ fn[K : Eq, V] Map::set_with_hash(
183192
curr_entry.value = value
184193
return
185194
}
186-
if psl > curr_entry.psl {
195+
if psl > self.psl_at(idx, curr_entry) {
187196
// Need to insert and push away - check if grow is needed first
188197
if self.size >= self.grow_at {
189198
self.grow()
190199
// Restart search with new capacity_mask
191200
continue 0, hash & self.capacity_mask
192201
}
193202
self.push_away(idx, curr_entry)
194-
let entry = { prev: self.tail, next: None, psl, key, value, hash }
203+
let entry = { prev: self.tail, next: None, key, value, hash }
195204
self.add_entry_to_tail(idx, entry)
196205
return
197206
}
@@ -209,20 +218,17 @@ fn[K, V] Map::push_away(
209218
entry : Entry[K, V],
210219
) -> Unit {
211220
// SAFETY: masked probe index; see the note at the top of this file.
212-
for psl = entry.psl + 1, idx = (idx + 1) & self.capacity_mask, entry = entry {
221+
for psl = self.psl_at(idx, entry) + 1, idx = (idx + 1) & self.capacity_mask, entry = entry {
213222
match self.entries.unsafe_get(idx) {
214223
None => {
215-
entry.psl = psl
216224
self.set_entry(entry, idx)
217225
break
218226
}
219227
Some(curr_entry) =>
220-
if psl > curr_entry.psl {
221-
entry.psl = psl
228+
if psl > self.psl_at(idx, curr_entry) {
229+
let displaced_psl = self.psl_at(idx, curr_entry) + 1
222230
self.set_entry(entry, idx)
223-
continue curr_entry.psl + 1,
224-
(idx + 1) & self.capacity_mask,
225-
curr_entry
231+
continue displaced_psl, (idx + 1) & self.capacity_mask, curr_entry
226232
} else {
227233
continue psl + 1, (idx + 1) & self.capacity_mask, entry
228234
}
@@ -274,7 +280,7 @@ pub fn[K : Hash + Eq, V] Map::get(self : Map[K, V], key : K) -> V? {
274280
if entry.hash == hash && entry.key == key {
275281
break Some(entry.value)
276282
}
277-
if i > entry.psl {
283+
if i > self.psl_at(idx, entry) {
278284
break None
279285
}
280286
continue i + 1, (idx + 1) & self.capacity_mask
@@ -292,7 +298,7 @@ pub fn[K : Hash + Eq, V] Map::at(self : Map[K, V], key : K) -> V {
292298
if entry.hash == hash && entry.key == key {
293299
return entry.value
294300
}
295-
guard! i <= entry.psl
301+
guard! i <= self.psl_at(idx, entry)
296302
continue i + 1, (idx + 1) & self.capacity_mask
297303
}
298304
}
@@ -333,7 +339,7 @@ pub fn[K : Hash + Eq, V] Map::get_or_default(
333339
if entry.hash == hash && entry.key == key {
334340
break entry.value
335341
}
336-
if i > entry.psl {
342+
if i > self.psl_at(idx, entry) {
337343
break default
338344
}
339345
continue i + 1, (idx + 1) & self.capacity_mask
@@ -352,14 +358,14 @@ pub fn[K : Hash + Eq, V] Map::get_or_init(
352358
) -> V {
353359
let hash = Hash::hash(key)
354360
// SAFETY: masked probe index; see the note at the top of this file.
355-
let (idx, psl, new_value, push_away) = for psl = 0, idx = hash &
356-
self.capacity_mask {
361+
let (idx, _, new_value, push_away) = for psl = 0, idx = hash &
362+
self.capacity_mask {
357363
match self.entries.unsafe_get(idx) {
358364
Some(entry) => {
359365
if entry.hash == hash && entry.key == key {
360366
return entry.value
361367
}
362-
if psl > entry.psl {
368+
if psl > self.psl_at(idx, entry) {
363369
let new_value = default()
364370
break (idx, psl, new_value, Some(entry))
365371
}
@@ -379,14 +385,7 @@ pub fn[K : Hash + Eq, V] Map::get_or_init(
379385
if push_away is Some(entry) {
380386
self.push_away(idx, entry)
381387
}
382-
let entry = {
383-
prev: self.tail,
384-
next: None,
385-
psl,
386-
hash,
387-
key,
388-
value: new_value,
389-
}
388+
let entry = { prev: self.tail, next: None, hash, key, value: new_value }
390389
self.add_entry_to_tail(idx, entry)
391390
}
392391
new_value
@@ -419,14 +418,14 @@ pub fn[K : Hash + Eq, V] Map::update_or_default(
419418
) -> Unit {
420419
let hash = Hash::hash(key)
421420
// SAFETY: masked probe index; see the note at the top of this file.
422-
let (idx, psl, push_away) = for psl = 0, idx = hash & self.capacity_mask {
421+
let (idx, _, push_away) = for psl = 0, idx = hash & self.capacity_mask {
423422
match self.entries.unsafe_get(idx) {
424423
Some(entry) => {
425424
if entry.hash == hash && entry.key == key {
426425
entry.value = f(entry.value)
427426
return
428427
}
429-
if psl > entry.psl {
428+
if psl > self.psl_at(idx, entry) {
430429
break (idx, psl, Some(entry))
431430
}
432431
continue psl + 1, (idx + 1) & self.capacity_mask
@@ -441,7 +440,7 @@ pub fn[K : Hash + Eq, V] Map::update_or_default(
441440
if push_away is Some(entry) {
442441
self.push_away(idx, entry)
443442
}
444-
let entry = { prev: self.tail, next: None, psl, hash, key, value: default }
443+
let entry = { prev: self.tail, next: None, hash, key, value: default }
445444
self.add_entry_to_tail(idx, entry)
446445
}
447446
}
@@ -457,7 +456,7 @@ pub fn[K : Hash + Eq, V] Map::contains(self : Map[K, V], key : K) -> Bool {
457456
if entry.hash == hash && entry.key == key {
458457
break true
459458
}
460-
if i > entry.psl {
459+
if i > self.psl_at(idx, entry) {
461460
break false
462461
}
463462
continue i + 1, (idx + 1) & self.capacity_mask
@@ -499,7 +498,7 @@ pub fn[K : Hash + Eq, V : Eq] Map::contains_kv(
499498
if entry.hash == hash && entry.key == key && entry.value == value {
500499
break true
501500
}
502-
if i > entry.psl {
501+
if i > self.psl_at(idx, entry) {
503502
break false
504503
}
505504
continue i + 1, (idx + 1) & self.capacity_mask
@@ -546,7 +545,7 @@ fn[K : Eq, V] Map::remove_with_hash(
546545
self.size -= 1
547546
break
548547
}
549-
if i > entry.psl {
548+
if i > self.psl_at(idx, entry) {
550549
break
551550
}
552551
continue i + 1, (idx + 1) & self.capacity_mask
@@ -592,15 +591,20 @@ fn[K, V] Map::shift_back(self : Map[K, V], idx : Int) -> Unit {
592591
for cur = idx {
593592
let next = (cur + 1) & self.capacity_mask
594593
match self.entries.unsafe_get(next) {
595-
None | Some({ psl: 0, .. }) => {
594+
None => {
596595
self.entries.unsafe_set(cur, None)
597596
break
598597
}
599-
Some(entry) => {
600-
entry.psl -= 1
601-
self.set_entry(entry, cur)
602-
continue next
603-
}
598+
Some(entry) =>
599+
if self.psl_at(next, entry) == 0 {
600+
self.entries.unsafe_set(cur, None)
601+
break
602+
} else {
603+
// Moving the entry one slot back drops its derived PSL by one, so
604+
// nothing has to be written for it.
605+
self.set_entry(entry, cur)
606+
continue next
607+
}
604608
}
605609
}
606610
}
@@ -637,15 +641,13 @@ fn[K, V] Map::rehash_place_entry(self : Map[K, V], outer : Entry[K, V]) -> Unit
637641
for psl = 0, idx = hash & self.capacity_mask {
638642
match self.entries.unsafe_get(idx) {
639643
None => {
640-
outer.psl = psl
641644
outer.prev = self.tail
642645
self.add_entry_to_tail(idx, outer)
643646
return
644647
}
645648
Some(curr) =>
646-
if psl > curr.psl {
649+
if psl > self.psl_at(idx, curr) {
647650
self.push_away(idx, curr)
648-
outer.psl = psl
649651
outer.prev = self.tail
650652
self.add_entry_to_tail(idx, outer)
651653
return
@@ -891,9 +893,9 @@ pub fn[K, V, V2] Map::map(self : Map[K, V], f : (K, V) -> V2) -> Map[K, V2] {
891893
}
892894
guard! self.entries[self.tail] is Some(last)
893895
for entry = last, idx = self.tail, next = (None : Entry[K, V2]?) {
894-
let { prev, psl, hash, key, value, .. } = entry
896+
let { prev, hash, key, value, .. } = entry
895897
let new_value = f(key, value)
896-
let new_entry = { prev, next, psl, hash, key, value: new_value }
898+
let new_entry = { prev, next, hash, key, value: new_value }
897899
other.entries[idx] = Some(new_entry)
898900
if prev != -1 {
899901
continue self.entries[prev].unwrap(), prev, Some(new_entry)
@@ -924,8 +926,8 @@ pub fn[K, V] Map::copy(self : Map[K, V]) -> Map[K, V] {
924926
}
925927
guard! self.entries[self.tail] is Some(last)
926928
for entry = last, idx = self.tail, next = (None : Entry[K, V]?) {
927-
let { prev, psl, hash, key, value, .. } = entry
928-
let new_entry = { prev, next, psl, hash, key, value }
929+
let { prev, hash, key, value, .. } = entry
930+
let new_entry = { prev, next, hash, key, value }
929931
other.entries[idx] = Some(new_entry)
930932
if prev != -1 {
931933
continue self.entries[prev].unwrap(), prev, Some(new_entry)
@@ -1135,8 +1137,8 @@ pub fn[K : Hash + Eq, V] Map::update(
11351137
) -> Unit {
11361138
let hash = Hash::hash(key)
11371139
// SAFETY: masked probe index; see the note at the top of this file.
1138-
let (idx, psl, new_value, push_away) = for psl = 0, idx = hash &
1139-
self.capacity_mask {
1140+
let (idx, _, new_value, push_away) = for psl = 0, idx = hash &
1141+
self.capacity_mask {
11401142
match self.entries.unsafe_get(idx) {
11411143
Some(entry) => {
11421144
if entry.hash == hash && entry.key == key {
@@ -1151,7 +1153,7 @@ pub fn[K : Hash + Eq, V] Map::update(
11511153
}
11521154
return
11531155
}
1154-
if psl > entry.psl {
1156+
if psl > self.psl_at(idx, entry) {
11551157
guard f(None) is Some(new_value) else { return }
11561158
break (idx, psl, new_value, Some(entry))
11571159
}
@@ -1171,14 +1173,7 @@ pub fn[K : Hash + Eq, V] Map::update(
11711173
if push_away is Some(entry) {
11721174
self.push_away(idx, entry)
11731175
}
1174-
let entry = {
1175-
prev: self.tail,
1176-
next: None,
1177-
psl,
1178-
hash,
1179-
key,
1180-
value: new_value,
1181-
}
1176+
let entry = { prev: self.tail, next: None, hash, key, value: new_value }
11821177
self.add_entry_to_tail(idx, entry)
11831178
}
11841179
}
@@ -1216,7 +1211,7 @@ pub fn[V] Map::get_from_bytes(map : Self[Bytes, V], key : BytesView) -> V? {
12161211
if entry.hash == hash && key.equal_to_bytes(entry.key) {
12171212
break Some(entry.value)
12181213
}
1219-
if i > entry.psl {
1214+
if i > map.psl_at(idx, entry) {
12201215
break None
12211216
}
12221217
continue i + 1, (idx + 1) & map.capacity_mask
@@ -1254,7 +1249,7 @@ pub fn[V] Map::get_from_string(map : Self[String, V], key : StringView) -> V? {
12541249
if entry.hash == hash && key.equal_to_string(entry.key) {
12551250
break Some(entry.value)
12561251
}
1257-
if i > entry.psl {
1252+
if i > map.psl_at(idx, entry) {
12581253
break None
12591254
}
12601255
continue i + 1, (idx + 1) & map.capacity_mask

builtin/linked_hash_map_wbtest.mbt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ test "remove_entry_head" {
585585
guard! map.head is Some(head)
586586
assert_true(head.prev == -1)
587587
assert_true(head.next is None)
588-
assert_true(head.psl == 0)
588+
assert_true(map.psl_at(head.hash & map.capacity_mask, head) == 0)
589589
assert_true(head.hash == (2).hash())
590590
assert_true(head.key == 2)
591591
assert_true(head.value == 2)
@@ -603,7 +603,7 @@ test "remove_entry_tail" {
603603
guard! map.entries[map.tail] is Some(tail)
604604
assert_true(tail.prev == -1)
605605
assert_true(tail.next is None)
606-
assert_true(tail.psl == 0)
606+
assert_true(map.psl_at(map.tail, tail) == 0)
607607
assert_true(tail.hash == (1).hash())
608608
assert_true(tail.key == 1)
609609
assert_true(tail.value == 1)
@@ -641,7 +641,8 @@ fn[K : Show, V : Show] Map::_debug_entries(self : Map[K, V]) -> String {
641641
}
642642
match entry {
643643
None => buf.write_char('_')
644-
Some({ psl, key, value, .. }) => buf <+ "(\{psl},\{key},\{value})"
644+
Some(entry) =>
645+
buf <+ "(\{self.psl_at(i, entry)},\{entry.key},\{entry.value})"
645646
}
646647
}
647648
buf.to_string()

0 commit comments

Comments
 (0)