Skip to content

Commit a8ba18b

Browse files
bobzhangclaude
andcommitted
perf(builtin): probe Map with unchecked array access
Every probe index in `Map` comes from `hash & capacity_mask` and is re-masked as `(idx + 1) & capacity_mask` on each step, so it is always in bounds for `entries`, whose length is `capacity`. The bounds check on those accesses is provably redundant. Convert the 18 masked-probe sites to `unsafe_get` / `unsafe_set`, with the invariant stated once at the top of the file and a marker at each loop head. `Map` is a linked hash map, so unlike `hashset` and `hashmap` it also stores slot indices in the list itself -- `prev`, `tail`, and the index `retain` destructures out of an entry. Every access through one of those keeps the checked form. Such indices are in bounds too, being former probe indices in a table that never shrinks, but that argument rests on the list being maintained correctly rather than on arithmetic alone, and that is not a foundation to put unchecked access on. `shift_back` is the deliberate exception. It is reachable from `retain` with a stored index, but `retain` performs its own checked read immediately before calling, so `shift_back` has a local caller-based proof that does not depend on the list invariant at all. It carries that proof at its definition, and that proof is also what makes the `set_entry` call inside it sound. Also adds `builtin/linked_hash_map_bench_test.mbt`, covering insertion, hits, misses, set+remove, and insertion-ordered iteration. Measured against `main`, n=50000, interleaved in one session: | backend | op | main | this | change | | ------- | -- | ---- | ---- | ------ | | js | `set` | 3.97 ms | 3.14 ms | 21% faster | | js | `get` hit | 1.30 ms | 1.02 ms | 22% faster | | js | `get` miss | 1.37 ms | 1.17 ms | 15% faster | | js | `set+remove` | 5.70 ms | 4.54 ms | 20% faster | | native | all | | | within noise | | wasm-gc | all | | | within noise | This is the largest js gain of the four hash containers, and the reason is load factor rather than anything about `Map` itself: at 50000 entries `Map` sits at roughly 76% of 65536 slots while `HashMap` sits at roughly 38% of 131072, so `Map` probes more per operation and therefore has more checks to remove. `HashSet`, at a similar load, gains similarly. The percentages are empirical -- js randomises its hash seed, so they will move between runs. `each` is unchanged by design, since it walks the list rather than the table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4e7043a commit a8ba18b

2 files changed

Lines changed: 154 additions & 18 deletions

File tree

builtin/linked_hash_map.mbt

Lines changed: 61 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,29 @@ struct Map[K, V] {
5151

5252
// Implementations
5353

54+
// SAFETY NOTE for the unchecked probe accesses in this file.
55+
//
56+
// `capacity` is always a power of two and at least 1, `capacity_mask` is
57+
// `capacity - 1`, and `entries.length()` is `capacity`. A probe index is
58+
// therefore always in bounds: it starts as `hash & capacity_mask` and each
59+
// step re-masks with `(idx + 1) & capacity_mask`. `grow` installs the new
60+
// array, capacity and mask together before any rehash probe runs, and
61+
// `set_with_hash` re-masks from scratch after growing.
62+
//
63+
// Only those probe indices use `unsafe_get` / `unsafe_set`. `Map` also
64+
// stores slot indices in the list itself -- `prev`, `tail`, and the index
65+
// `retain` destructures out of an entry -- and every access through one of
66+
// those keeps the checked form. They are in bounds too, being former probe
67+
// indices in a table that never shrinks, but that argument depends on the
68+
// list being maintained correctly rather than on arithmetic alone, so it is
69+
// not one to build unchecked access on.
70+
//
71+
// `shift_back` is the exception, and deliberately so: it is reachable from
72+
// `retain` with a stored index, but `retain` performs its own checked read
73+
// immediately before calling, so `shift_back` has a local caller-based proof
74+
// that does not depend on the list invariant. It carries that proof at its
75+
// definition, and it is what makes its `set_entry` call sound.
76+
5477
///|
5578
let default_init_capacity = 8
5679

@@ -140,8 +163,9 @@ fn[K : Eq, V] Map::set_with_hash(
140163
hash : Int,
141164
) -> Unit {
142165
// Only grow when actually inserting a new entry, not when updating existing
166+
// SAFETY: masked probe index; see the note at the top of this file.
143167
for psl = 0, idx = hash & self.capacity_mask {
144-
match self.entries[idx] {
168+
match self.entries.unsafe_get(idx) {
145169
None => {
146170
// Need to insert new entry - check if grow is needed first
147171
if self.size >= self.grow_at {
@@ -184,8 +208,9 @@ fn[K, V] Map::push_away(
184208
idx : Int,
185209
entry : Entry[K, V],
186210
) -> Unit {
211+
// SAFETY: masked probe index; see the note at the top of this file.
187212
for psl = entry.psl + 1, idx = (idx + 1) & self.capacity_mask, entry = entry {
188-
match self.entries[idx] {
213+
match self.entries.unsafe_get(idx) {
189214
None => {
190215
entry.psl = psl
191216
self.set_entry(entry, idx)
@@ -219,7 +244,7 @@ fn[K, V] Map::set_entry(
219244
None => self.tail = new_idx
220245
Some(next) => next.prev = new_idx
221246
}
222-
self.entries[new_idx] = Some(entry)
247+
self.entries.unsafe_set(new_idx, Some(entry))
223248
}
224249

225250
///|
@@ -243,8 +268,9 @@ fn[K, V] Map::set_entry(
243268
/// ```
244269
pub fn[K : Hash + Eq, V] Map::get(self : Map[K, V], key : K) -> V? {
245270
let hash = Hash::hash(key)
271+
// SAFETY: masked probe index; see the note at the top of this file.
246272
for i = 0, idx = hash & self.capacity_mask {
247-
guard self.entries[idx] is Some(entry) else { break None }
273+
guard self.entries.unsafe_get(idx) is Some(entry) else { break None }
248274
if entry.hash == hash && entry.key == key {
249275
break Some(entry.value)
250276
}
@@ -260,8 +286,9 @@ pub fn[K : Hash + Eq, V] Map::get(self : Map[K, V], key : K) -> V? {
260286
#alias("_[_]")
261287
pub fn[K : Hash + Eq, V] Map::at(self : Map[K, V], key : K) -> V {
262288
let hash = Hash::hash(key)
289+
// SAFETY: masked probe index; see the note at the top of this file.
263290
for i = 0, idx = hash & self.capacity_mask {
264-
guard! self.entries[idx] is Some(entry)
291+
guard! self.entries.unsafe_get(idx) is Some(entry)
265292
if entry.hash == hash && entry.key == key {
266293
return entry.value
267294
}
@@ -299,8 +326,9 @@ pub fn[K : Hash + Eq, V] Map::get_or_default(
299326
default : V,
300327
) -> V {
301328
let hash = Hash::hash(key)
329+
// SAFETY: masked probe index; see the note at the top of this file.
302330
for i = 0, idx = hash & self.capacity_mask {
303-
match self.entries[idx] {
331+
match self.entries.unsafe_get(idx) {
304332
Some(entry) => {
305333
if entry.hash == hash && entry.key == key {
306334
break entry.value
@@ -323,9 +351,10 @@ pub fn[K : Hash + Eq, V] Map::get_or_init(
323351
default : () -> V,
324352
) -> V {
325353
let hash = Hash::hash(key)
354+
// SAFETY: masked probe index; see the note at the top of this file.
326355
let (idx, psl, new_value, push_away) = for psl = 0, idx = hash &
327356
self.capacity_mask {
328-
match self.entries[idx] {
357+
match self.entries.unsafe_get(idx) {
329358
Some(entry) => {
330359
if entry.hash == hash && entry.key == key {
331360
return entry.value
@@ -389,8 +418,9 @@ pub fn[K : Hash + Eq, V] Map::update_or_default(
389418
f : (V) -> V,
390419
) -> Unit {
391420
let hash = Hash::hash(key)
421+
// SAFETY: masked probe index; see the note at the top of this file.
392422
let (idx, psl, push_away) = for psl = 0, idx = hash & self.capacity_mask {
393-
match self.entries[idx] {
423+
match self.entries.unsafe_get(idx) {
394424
Some(entry) => {
395425
if entry.hash == hash && entry.key == key {
396426
entry.value = f(entry.value)
@@ -421,8 +451,9 @@ pub fn[K : Hash + Eq, V] Map::update_or_default(
421451
pub fn[K : Hash + Eq, V] Map::contains(self : Map[K, V], key : K) -> Bool {
422452
// inline Map::get to avoid boxing
423453
let hash = Hash::hash(key)
454+
// SAFETY: masked probe index; see the note at the top of this file.
424455
for i = 0, idx = hash & self.capacity_mask {
425-
guard self.entries[idx] is Some(entry) else { break false }
456+
guard self.entries.unsafe_get(idx) is Some(entry) else { break false }
426457
if entry.hash == hash && entry.key == key {
427458
break true
428459
}
@@ -462,8 +493,9 @@ pub fn[K : Hash + Eq, V : Eq] Map::contains_kv(
462493
) -> Bool {
463494
// inline Map::get to avoid boxing
464495
let hash = Hash::hash(key)
496+
// SAFETY: masked probe index; see the note at the top of this file.
465497
for i = 0, idx = hash & self.capacity_mask {
466-
guard self.entries[idx] is Some(entry) else { break false }
498+
guard self.entries.unsafe_get(idx) is Some(entry) else { break false }
467499
if entry.hash == hash && entry.key == key && entry.value == value {
468500
break true
469501
}
@@ -505,8 +537,9 @@ fn[K : Eq, V] Map::remove_with_hash(
505537
key : K,
506538
hash : Int,
507539
) -> Unit {
540+
// SAFETY: masked probe index; see the note at the top of this file.
508541
for i = 0, idx = hash & self.capacity_mask {
509-
guard self.entries[idx] is Some(entry) else { break }
542+
guard self.entries.unsafe_get(idx) is Some(entry) else { break }
510543
if entry.hash == hash && entry.key == key {
511544
self.remove_entry(entry)
512545
self.shift_back(idx)
@@ -532,7 +565,7 @@ fn[K, V] Map::add_entry_to_tail(
532565
tail => self.entries[tail].unwrap().next = Some(entry)
533566
}
534567
self.tail = idx
535-
self.entries[idx] = Some(entry)
568+
self.entries.unsafe_set(idx, Some(entry))
536569
self.size += 1
537570
}
538571

@@ -550,11 +583,17 @@ fn[K, V] Map::remove_entry(self : Map[K, V], entry : Entry[K, V]) -> Unit {
550583

551584
///|
552585
fn[K, V] Map::shift_back(self : Map[K, V], idx : Int) -> Unit {
586+
// SAFETY: the initial `cur` is in bounds by every route its callers take,
587+
// and none of them requires trusting the list invariant: `remove` and
588+
// `update` pass a masked probe index, and `retain` performs its own
589+
// checked `entries[idx]` read immediately before calling. `next` is
590+
// re-masked each step and later `cur` values are previous `next` values.
591+
// This is also what makes the `set_entry` call below sound.
553592
for cur = idx {
554593
let next = (cur + 1) & self.capacity_mask
555-
match self.entries[next] {
594+
match self.entries.unsafe_get(next) {
556595
None | Some({ psl: 0, .. }) => {
557-
self.entries[cur] = None
596+
self.entries.unsafe_set(cur, None)
558597
break
559598
}
560599
Some(entry) => {
@@ -594,8 +633,9 @@ fn[K, V] Map::grow(self : Map[K, V]) -> Unit {
594633
#owned(outer)
595634
fn[K, V] Map::rehash_place_entry(self : Map[K, V], outer : Entry[K, V]) -> Unit {
596635
let hash = outer.hash
636+
// SAFETY: masked probe index; see the note at the top of this file.
597637
for psl = 0, idx = hash & self.capacity_mask {
598-
match self.entries[idx] {
638+
match self.entries.unsafe_get(idx) {
599639
None => {
600640
outer.psl = psl
601641
outer.prev = self.tail
@@ -1094,9 +1134,10 @@ pub fn[K : Hash + Eq, V] Map::update(
10941134
f : (V?) -> V?,
10951135
) -> Unit {
10961136
let hash = Hash::hash(key)
1137+
// SAFETY: masked probe index; see the note at the top of this file.
10971138
let (idx, psl, new_value, push_away) = for psl = 0, idx = hash &
10981139
self.capacity_mask {
1099-
match self.entries[idx] {
1140+
match self.entries.unsafe_get(idx) {
11001141
Some(entry) => {
11011142
if entry.hash == hash && entry.key == key {
11021143
// Found the entry, update its value
@@ -1169,8 +1210,9 @@ pub fn[K : Hash + Eq, V] Map::update(
11691210
/// ```
11701211
pub fn[V] Map::get_from_bytes(map : Self[Bytes, V], key : BytesView) -> V? {
11711212
let hash = key.hash()
1213+
// SAFETY: masked probe index; see the note at the top of this file.
11721214
for i = 0, idx = hash & map.capacity_mask {
1173-
guard map.entries[idx] is Some(entry) else { break None }
1215+
guard map.entries.unsafe_get(idx) is Some(entry) else { break None }
11741216
if entry.hash == hash && key.equal_to_bytes(entry.key) {
11751217
break Some(entry.value)
11761218
}
@@ -1206,8 +1248,9 @@ pub fn[V] Map::get_from_bytes(map : Self[Bytes, V], key : BytesView) -> V? {
12061248
/// ```
12071249
pub fn[V] Map::get_from_string(map : Self[String, V], key : StringView) -> V? {
12081250
let hash = key.hash()
1251+
// SAFETY: masked probe index; see the note at the top of this file.
12091252
for i = 0, idx = hash & map.capacity_mask {
1210-
guard map.entries[idx] is Some(entry) else { break None }
1253+
guard map.entries.unsafe_get(idx) is Some(entry) else { break None }
12111254
if entry.hash == hash && key.equal_to_string(entry.key) {
12121255
break Some(entry.value)
12131256
}
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// Copyright 2026 International Digital Economy Academy
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
///|
16+
let map_bench_n = 50000
17+
18+
///|
19+
test "bench Map::set n=50000" (it : @bench.T) {
20+
it.bench(fn() {
21+
let m : Map[Int, Int] = Map([])
22+
for i in 0..<map_bench_n {
23+
m.set(i * 1103515245, i)
24+
}
25+
it.keep(m.length())
26+
})
27+
}
28+
29+
///|
30+
test "bench Map::get n=50000" (it : @bench.T) {
31+
let m : Map[Int, Int] = Map([])
32+
for i in 0..<map_bench_n {
33+
m.set(i * 1103515245, i)
34+
}
35+
it.bench(fn() {
36+
let mut hits = 0
37+
for i in 0..<map_bench_n {
38+
if m.get(i * 1103515245) is Some(_) {
39+
hits += 1
40+
}
41+
}
42+
it.keep(hits)
43+
})
44+
}
45+
46+
///|
47+
test "bench Map::get miss n=50000" (it : @bench.T) {
48+
let m : Map[Int, Int] = Map([])
49+
for i in 0..<map_bench_n {
50+
m.set(i * 1103515245, i)
51+
}
52+
it.bench(fn() {
53+
let mut misses = 0
54+
for i in 0..<map_bench_n {
55+
if m.get(i * 1103515245 + 1) is None {
56+
misses += 1
57+
}
58+
}
59+
it.keep(misses)
60+
})
61+
}
62+
63+
///|
64+
/// Named for what it measures: `@bench.T` has no per-iteration setup hook,
65+
/// so the map has to be rebuilt inside the timed closure and the removal
66+
/// cost cannot be isolated from the insertion cost.
67+
test "bench Map::set+remove n=50000" (it : @bench.T) {
68+
it.bench(fn() {
69+
let m : Map[Int, Int] = Map([])
70+
for i in 0..<map_bench_n {
71+
m.set(i * 1103515245, i)
72+
}
73+
for i in 0..<map_bench_n {
74+
m.remove(i * 1103515245)
75+
}
76+
it.keep(m.length())
77+
})
78+
}
79+
80+
///|
81+
/// Insertion-ordered iteration, which is the reason `Map` keeps a list at
82+
/// all, so it is worth guarding against regressions here.
83+
test "bench Map::each n=50000" (it : @bench.T) {
84+
let m : Map[Int, Int] = Map([])
85+
for i in 0..<map_bench_n {
86+
m.set(i * 1103515245, i)
87+
}
88+
it.bench(fn() {
89+
let mut total = 0
90+
m.each((_, v) => total = total + v)
91+
it.keep(total)
92+
})
93+
}

0 commit comments

Comments
 (0)