perf(hashset): store the table as two arrays, deriving each PSL

[bobzhang]

Supersedes #4127, which I have marked draft. Same goal — stop allocating a boxed Entry per key — but without the js regression that made #4127 unlandable.

The design is Codex CLI's, proposed while explaining that regression. I implemented and measured it.

What changed

#4127 stored three arrays: psls, hashes, keys. A probe step read all three, at the same index in three arrays 512 KB apart, so it touched three cache lines where the boxed layout touched one — the entry object holds psl, hash and key together. That cost js insertion 15% growing and 44% pre-sized.

A slot's probe-sequence length is not independent state. It is the distance between where the key sits and where its hash asked it to sit:

psl = (idx - (stored_hash & mask)) & mask

So psls goes away. Two arrays remain, and a probe step trades a random load in a 512 KB array for two arithmetic operations.

hashes[i] == 0 marks an empty slot. Any Int can be a real hash, so store_hash remaps 0 onto 1; the two then share a bucket, which costs an occasional extra key comparison and nothing else, since lookups compare keys as well as hashes.

shift_back no longer writes a PSL at all — moving an occupant one slot back is the decrement.

Verifying the premise before building

psl was a mut field, which suggests independent state. I checked rather than assumed: I instrumented set_slot on the three-array branch to assert psl == (idx - (hash & mask)) & mask on every write, then ran the full suite on three backends plus a four-bucket collision stress with removals, shift-back cascades, growth and copy. It never fired. Every PSL write happened as a key landed in a slot, always equal to its displacement.

Measurements

n=50000, against main, interleaved on one machine in a single session:

backend	op	main	this branch	change
native	add	2.23 ms	1.42 ms	36% faster
native	contains	842 µs	744 µs	12% faster
native	copy	484 µs	65 µs	86% faster
native	add string	3.51 ms	3.22 ms	8% faster
native	contains string	2.16 ms	1.84 ms	15% faster
wasm-gc	add	2.20 ms	1.78 ms	19% faster
wasm-gc	contains	1.06 ms	987 µs	7% faster
wasm-gc	copy	373 µs	168 µs	55% faster
wasm-gc	add string	3.36 ms	3.00 ms	11% faster
wasm-gc	contains string	2.61 ms	2.41 ms	8% faster
js	add	2.37 ms	2.33 ms	unchanged
js	contains	962 µs	904 µs	6% faster
js	copy	432 µs	314 µs	27% faster
js	add string	3.41 ms	3.58 ms	5% slower
js	contains string	2.40 ms	2.21 ms	8% faster

Faster on fourteen of fifteen. The two js insertion figures are the mean of two alternating samples per side, since they are what the design turns on: integer add came out 2.32/2.33 against 2.38/2.36, string add 3.57/3.59 against 3.42/3.39.

String keys are benchmarked because the boxed layout stores a pointer inside its entry where this one stores it directly in the key array — a difference integer benchmarks cannot show. That case is the single remaining regression, at 5%.

Tests

The derivation is now load-bearing rather than merely true, so derived_psl_test.mbt covers the paths that used to write a PSL: a four-bucket collision stress with removal and reinsertion across six growth rounds, keys whose hash is the remapped sentinel, and a 20000-operation differential run against a bitmap model.

Review

Verified by Codex CLI at xhigh. It found no algorithmic defect — psl_at correct across wraparound, shift_back's implicit decrement exact, push_away preserving the Robin Hood recurrence and terminating, growth reinserting under the new mask, the sentinel remap sound, every psl_at call guarded by an occupied slot, and miss lookup retaining its early exit. Full verdict in a comment below.

It withheld a sign-off on methodology rather than correctness: its sandbox is read-only, so it could not run fresh interleaved benchmarks and would not certify numbers it had not reproduced. The alternating samples above are my response to that; the benchmark claims remain mine.

One gap it names and I have not closed: deleting keys.set_null(cur) in shift_back would leave every test passing while removed keys stayed reachable. That is the same liveness gap discussed on #3712 — no portable way to assert it across four backends — so the guard is a comment at the line explaining why it is not redundant.

[bobzhang]

Codex CLI verification (xhigh)

No algorithmic defect found. It withheld sign-off on methodology — a read-only sandbox prevented it running fresh benchmarks — not on correctness.

I would not land it yet. I found no algorithmic correctness defect, but the required fresh benchmark comparison and full validation could not be completed because the workspace is read-only: Moon cannot acquire its _build lock.

Algorithm review:

• psl_at correctly computes circular displacement modulo the power-of-two capacity.
• In shift_back, moving next to cur = next - 1 mod capacity reduces its derived PSL by exactly one, including wraparound. Stopping at an empty slot or PSL zero matches the old implementation.
• push_away preserves the previous Robin Hood recurrence exactly. It must terminate because insertion occurs with at least one empty slot; growth caps occupancy at 13/16.
• Growth reinserts every occupied entry using the new mask, so no old-table displacement information is needed.
• Mapping hashes 0 → 1 is sound. Real hashes zero and one share their home bucket and cached fingerprint, but equality comparisons distinguish their keys. Insertion, lookup, removal, and growth all use the remapped value consistently.
• Every psl_at call is guarded by an occupied hash slot or receives an occupant directly from such a slot.
• Miss lookup retains the same Robin Hood early exit. Independently, an empty slot must occur before a full-table scan because the table is never close to full.

Test mutation not caught: remove self.keys.set_null(cur). Membership tests would still pass because the hash sentinel controls occupancy, but removed reference keys would remain retained.

• git diff --check passed. hashset/pkg.generated.mbti is unchanged from origin/main. Direct formatter comparison found all four changed files formatted. Cached native and JS executables passed the derived-PSL and internal wraparound tests.

Benchmark conclusion: undetermined. […] there is no same-session origin/main baseline, and those binaries predate the commit by several minutes. Therefore they cannot establish either parity or a real string regression.

That last point is the right call and I have answered it with alternating samples rather than a single pass, since the two js insertion figures are what the whole design turns on:

js, n=50000	two-array (A1, A2)	main (B1, B2)
add integer keys	2.32, 2.33 ms	2.38, 2.36 ms
add string keys	3.57, 3.59 ms	3.42, 3.39 ms

Integer insertion is at parity — marginally ahead, with ranges overlapping. String insertion is a real 5% regression, which is tighter than the 7% I first reported from a single sample.

The uncaught mutation is the same liveness gap discussed on #3712: asserting that a removed key became collectable needs weak references or finalization, which is not portable across the four backends. The guard is a comment at the line, since what actually threatens that call is someone reading shift_back and deleting a line that looks redundant beside the hashes write above it.

moon check --deny-warn clean on native, js and wasm-gc; moon test 7549/7549 and 7490/7490 on js; pkg.generated.mbti unchanged.

[Copilot]

Pull request overview

Refactors hashset’s internal table layout to eliminate per-entry allocations by switching from boxed Entry objects to two parallel arrays (hashes + keys) and deriving probe-sequence length (PSL) from (idx - home_idx) & mask. This targets better cache behavior and lower allocation overhead while preserving the Robin Hood hashing behavior.

Changes:

• Replace FixedArray[Entry[K]?] with FixedArray[Int] (hashes) + UninitializedArray[K] (keys) and derive PSL via psl_at.
• Introduce empty_hash sentinel handling with store_hash remapping and update all core operations (add, contains, remove, shift_back, grow, copy) accordingly.
• Add targeted regression tests for derived PSL behavior and extend benchmarks (copy/remove, string-key cases).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
hashset/types.mbt	Introduces sentinel hash scheme, store_hash, and package-local unchecked key-array accessors; updates HashSet storage fields.
hashset/hashset.mbt	Rewrites HashSet operations to use (hashes, keys) layout and derived PSL; adds/updates tests and optimizes copy/clear.
hashset/hashset_bench_test.mbt	Adds copy/remove benchmarks and string-key benchmarks to validate perf claims and cover removal path.
hashset/derived_psl_test.mbt	New stress + differential tests to validate derived PSL across collisions, removals, growth, copying, and sentinel remap.

Suppressed comments (1)

hashset/hashset.mbt:836

• Safety comment still mentions a “non-empty PSL”, but PSLs are derived now. Use the actual occupancy predicate (hashes[i] != empty_hash) so the invariant is accurate.

    // SAFETY: `i < capacity`, the length of every backing array, and a
    // non-empty PSL means the key slot was initialized.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coveralls]

Coverage Report for CI Build 6300

Coverage increased (+0.008%) to 90.885%

Details

• Coverage increased (+0.008%) from the base build.
• Patch coverage: 5 uncovered changes across 1 file (72 of 77 lines covered, 93.51%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
hashset/hashset.mbt	74	69	93.24%
Total (2 files)	77	72	93.51%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	18464
Covered Lines:	16781
Line Coverage:	90.88%
Coverage Strength:	306565.25 hits per line

---

💛 - Coveralls