From af6309a60dd372c3c2bbd0b9b246d4de211e97cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 13:46:21 +0200 Subject: [PATCH 1/3] perf(shapes): stop paying SipHash on every ShapeId probe, and make the id a shape resolves to hash-order-independent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ShapeTableInner::descriptors` — the map `shape_descriptor_by_id` reads — was a `std::collections::HashMap` with the default `RandomState`, i.e. SipHash-1-3 on a bare `u32`. That probe is the hottest lookup in the object model: `object_is_regular` runs it once per array element-shape test (3 M times on the `retain` bench, 20 M on `churn`) and `object_live_slot_count` runs it on every indexed field get/set. A symbol profile of the `shapes` bench (`CARGO_PROFILE_RELEASE_STRIP=none` + `PERRY_DEBUG_SYMBOLS=1` + `sample`) put `RandomState::hash_one` at the TOP of self time, with `shape_descriptor_by_id` fourth — together ~22% of the program. The sibling field on the very same struct, `indices`, already used `crate::fast_hash::PtrHashMap`; `descriptors` and `ids_by_keys` were simply never converted. `fast_hash`'s own module doc records the identical finding ("`hash_one` was 14% leaf samples") for the pointer-keyed registries. Two parts: * `descriptors` and `ids_by_keys` become `PtrHashMap`, and `PtrHasherImpl` gains a `write_u32` fast path — without it a `u32` key falls into the generic byte-stream fallback (`Hasher`'s default `write_u32` forwards to `write(&n.to_ne_bytes())`), four rotate/xor rounds for a key that needs one multiply. `ids_by_facts` is deliberately NOT converted: `PtrHasher`'s `write_*` methods overwrite the accumulator rather than folding it, which is right for a one-word key and wrong for that five-field one. * `rebuild_descriptor_reverse_indices` now sorts each id vector. The rebuild walks `descriptors` in HASH order and `shape_descriptor_ensure_with_generation` reuses `ids.first()`, so WHICH ShapeId a facts key resolves to after a GC rewrite depended on the hasher. Two objects with identical facts, one born before a collection and one after, then carry different ids and every id-keyed consumer (the typed shape-layout install, the emitted PICs) splits its population. This was latent, and swapping the hasher exposed it: without the sort the hasher change alone measured interp +3.4% / pipeline +0.5%, with `gc::layout::init_typed_shape_layout` and `gc::shape_install::record` newly hot in the profile. With it, interp is -5.9%. Measured on the 19-program corpus, `/usr/bin/time -l`, best-of-3, both arms built with an identical `-p` set and `PERRY_RUNTIME_DIR` pinned per arm, the two `libperry_runtime.a` files `cmp`-verified to differ, all 19 stdouts byte-compared and exit-checked: churn -25.2% deeplist -17.2% retain_wide -15.2% retain1 -15.1% cycles -14.9% retain -14.7% retain_wide1 -14.3% tree -13.4% shapes -9.4% tree_wide -6.5% interp -5.9% iso_miss -4.7% pipeline -3.6% asyncpipe -1.1% fib40/push_num/churn_read ~0 Peak memory footprint is unchanged on every row (<=0.1% except run-to-run noise on the two smallest). No behavioural change: all 19 programs produce byte-identical stdout. Refs #8125, #8113, #8122. --- crates/perry-runtime/src/fast_hash.rs | 56 +++++++++++++++++++++++ crates/perry-runtime/src/object/shapes.rs | 51 ++++++++++++++++++--- 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/crates/perry-runtime/src/fast_hash.rs b/crates/perry-runtime/src/fast_hash.rs index 90aaa2ce7d..bff7e1b82b 100644 --- a/crates/perry-runtime/src/fast_hash.rs +++ b/crates/perry-runtime/src/fast_hash.rs @@ -54,6 +54,19 @@ impl Hasher for PtrHasherImpl { } self.0 = mix(h.wrapping_mul(PTR_MIX)); } + /// #8125: `u32` keys must not fall into the byte-stream fallback above. + /// `Hash for u32` calls `write_u32`, and `Hasher`'s DEFAULT `write_u32` + /// forwards to `write(&n.to_ne_bytes())` — four rotate/xor iterations plus + /// the multiply, for a key that needs exactly one multiply. The shape + /// descriptor table (`object::shapes`) is keyed by a bare `u32` ShapeId and + /// is probed once per allocated object and once per array element-shape + /// test, so this override is the difference between a fast path and a + /// loop. Deliberately identical to `write_u64` on the same numeric value; + /// `u32_keys_take_the_multiplicative_fast_path` pins that. + #[inline] + fn write_u32(&mut self, n: u32) { + self.0 = mix((n as u64).wrapping_mul(PTR_MIX)); + } #[inline] fn write_u64(&mut self, n: u64) { self.0 = mix(n.wrapping_mul(PTR_MIX)); @@ -183,6 +196,49 @@ mod tests { assert_eq!(m.get(&0x9999), None); } + /// #8125: a `u32` key must reach the single-multiply path, not the + /// per-byte fold. `Hash for u32` calls `write_u32`; without an override + /// `Hasher`'s default `write_u32` forwards to `write(&n.to_ne_bytes())`. + /// Deleting `PtrHasherImpl::write_u32` turns this red. + #[test] + fn u32_keys_take_the_multiplicative_fast_path() { + use std::hash::Hash; + for n in [0u32, 1, 0x8000_0000, 0x8000_02ff, u32::MAX] { + let mut viaHash = PtrHasher.build_hasher(); + n.hash(&mut viaHash); + let mut viaU64 = PtrHasher.build_hasher(); + viaU64.write_u64(n as u64); + assert_eq!( + viaHash.finish(), + viaU64.finish(), + "u32 key {n:#x} fell into the byte-stream fallback" + ); + } + } + + /// ShapeIds are minted sequentially from `SHAPE_ID_BASE` (0x8000_0000), so + /// the descriptor table's keys are a dense run in the TOP half of the u32 + /// range. Multiplicative mixing has to spread that run across buckets — + /// `HashMap` indexes on the LOW bits, and a run of consecutive integers has + /// no entropy there. Dropping `mix` (the `^= h >> 32` avalanche) collapses + /// this to a handful of buckets and turns the map into a linked list. + #[test] + fn sequential_shape_ids_spread_across_low_bit_buckets() { + use std::collections::HashSet; + let base = 0x8000_0000u32; + let mut buckets = HashSet::new(); + for i in 0..1024u32 { + let mut h = PtrHasher.build_hasher(); + h.write_u32(base + i); + buckets.insert(h.finish() & 0x3ff); + } + assert!( + buckets.len() > 512, + "sequential ShapeIds hashed into only {} of 1024 buckets", + buckets.len() + ); + } + /// Pointer-aligned keys collide trivially with multiply-only on the /// low bits — Fibonacci-hash mixing into the upper bits is what /// keeps the buckets evenly populated. Sanity-check that 1000 8-byte- diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 430e747032..a57d1d543a 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -72,14 +72,36 @@ struct ShapeFacts { struct ShapeTableInner { indices: crate::fast_hash::PtrHashMap, - descriptors: HashMap, + /// #8125: `PtrHashMap`, not the SipHash default. + /// + /// This is the map `shape_descriptor_by_id` probes, and that probe is the + /// single hottest runtime lookup in the object model: `object_is_regular` + /// runs it once per array element-shape test (3 M times on the `retain` + /// bench, 20 M on `churn`) and, since #8113 deleted + /// `ObjectHeader::field_count`, `object_live_slot_count` runs it on every + /// indexed field get/set. A symbol profile of the `shapes` bench + /// (`PERRY_DEBUG_SYMBOLS=1` + `sample`) put `RandomState::hash_one` at the + /// TOP of self time with `shape_descriptor_by_id` fourth — together ~22% of + /// the program, nearly all of it SipHash on a bare `u32`. + /// + /// The key is a ShapeId minted by this process from a monotonic counter. + /// No external input reaches it, so hash-flooding resistance buys nothing + /// here for the same reason it buys nothing on the pointer-keyed + /// registries `fast_hash` already serves. + descriptors: crate::fast_hash::PtrHashMap, /// Exact-facts reverse index. More than one id is legal when a worker /// minted a local descriptor before a process-global module id arrived. + /// + /// Deliberately NOT a `PtrHashMap`: `PtrHasher`'s `write_*` methods + /// OVERWRITE the accumulator instead of folding it, which is exactly right + /// for a single-word key and wrong for this five-field one — every + /// `ShapeFacts` would hash to its last field alone. ids_by_facts: HashMap>, /// Keys-array address -> every descriptor id that currently names it. /// Same-address key-count retirement uses this index instead of scanning - /// every shape ever observed by the agent. - ids_by_keys: HashMap>, + /// every shape ever observed by the agent. Single-word key, so `PtrHasher` + /// (#8125). + ids_by_keys: crate::fast_hash::PtrHashMap>, } pub(crate) struct ShapeTable { @@ -91,9 +113,9 @@ impl ShapeTable { ShapeTable { inner: RefCell::new(ShapeTableInner { indices: crate::fast_hash::new_ptr_hash_map(), - descriptors: HashMap::new(), + descriptors: crate::fast_hash::new_ptr_hash_map(), ids_by_facts: HashMap::new(), - ids_by_keys: HashMap::new(), + ids_by_keys: crate::fast_hash::new_ptr_hash_map(), }), } } @@ -137,7 +159,8 @@ fn remove_id_from_facts_index(inner: &mut ShapeTableInner, facts: ShapeFacts, id fn rebuild_descriptor_reverse_indices(inner: &mut ShapeTableInner) { let mut ids_by_facts: HashMap> = HashMap::with_capacity(inner.descriptors.len()); - let mut ids_by_keys: HashMap> = HashMap::new(); + let mut ids_by_keys: crate::fast_hash::PtrHashMap> = + crate::fast_hash::new_ptr_hash_map(); for (&id, &descriptor) in &inner.descriptors { ids_by_facts .entry(descriptor_facts(descriptor)) @@ -145,6 +168,22 @@ fn rebuild_descriptor_reverse_indices(inner: &mut ShapeTableInner) { .push(id); ids_by_keys.entry(descriptor.keys).or_default().push(id); } + // #8125: the rebuild walks `descriptors` in HASH order, and + // `shape_descriptor_ensure_with_generation` reuses `ids.first()` — so + // without this sort, WHICH id a facts key resolves to after a GC rewrite + // depends on the hasher. Two objects with identical facts, one born before + // a collection and one after, would then carry different ShapeIds, and + // every id-keyed consumer (the typed shape-layout install, the emitted + // PICs) splits its population. Ascending is the stable canonical choice: + // ids are minted monotonically, so the smallest is the oldest — the one + // already-published objects and already-installed layouts carry, and in + // practice the module-init id `install_external_shape_id` prefers. + for ids in ids_by_facts.values_mut() { + ids.sort_unstable(); + } + for ids in ids_by_keys.values_mut() { + ids.sort_unstable(); + } inner.ids_by_facts = ids_by_facts; inner.ids_by_keys = ids_by_keys; } From f21a92e2bfb4eab42c29ffdf8d7f0bf446dc8ba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 13:49:31 +0200 Subject: [PATCH 2/3] docs(changelog): fragment for #8157 --- changelog.d/8157-shape-descriptor-siphash.md | 59 ++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 changelog.d/8157-shape-descriptor-siphash.md diff --git a/changelog.d/8157-shape-descriptor-siphash.md b/changelog.d/8157-shape-descriptor-siphash.md new file mode 100644 index 0000000000..bc0db96430 --- /dev/null +++ b/changelog.d/8157-shape-descriptor-siphash.md @@ -0,0 +1,59 @@ +### Performance + +**The ShapeId descriptor table no longer pays SipHash on every probe, and the +id a shape resolves to no longer depends on hash iteration order.** + +`ShapeTableInner::descriptors` — the map `shape_descriptor_by_id` reads — was a +`std::collections::HashMap` with the default `RandomState`, i.e. +SipHash-1-3 on a bare `u32`. It is the hottest lookup in the object model: +`object_is_regular` runs it once per array element-shape test (3,000,000 times +on the `retain` bench, 20,000,002 on `churn`) and `object_live_slot_count` runs +it on every indexed field get/set. The sibling field on the same struct, +`indices`, already used `crate::fast_hash::PtrHashMap`; `descriptors` and +`ids_by_keys` were simply never converted, and `fast_hash`'s own module doc +already records the identical finding ("`hash_one` was 14% leaf samples") for +the pointer-keyed registries. + +`PtrHasherImpl` gains a `write_u32` fast path — without it a `u32` key falls +into the generic byte-stream fallback, because `Hasher`'s default `write_u32` +forwards to `write(&n.to_ne_bytes())`: four rotate/xor rounds for a key that +needs one multiply. `ids_by_facts` is deliberately NOT converted, since +`PtrHasher`'s `write_*` methods overwrite the accumulator rather than folding +it — right for a one-word key, wrong for that five-field one. + +`rebuild_descriptor_reverse_indices` now sorts each id vector. The rebuild +walks `descriptors` in HASH order and `shape_descriptor_ensure_with_generation` +reuses `ids.first()`, so which ShapeId a facts key resolved to after a GC +rewrite depended on the hasher: two objects with identical facts, one born +before a collection and one after, carried different ids, and every id-keyed +consumer (the typed shape-layout install, the emitted PICs) split its +population. Latent before, exposed by the hasher swap — which alone measured +`interp` +3.4% with `gc::layout::init_typed_shape_layout` newly doubled in the +profile, and `interp` −5.9% once the order was pinned. + +Measured on the 19-program corpus, `/usr/bin/time -l`, best-of-3, per-arm +`PERRY_RUNTIME_DIR` and object cache, archives `cmp`-verified to differ, all 19 +stdouts byte-compared and exit-checked: + +``` +churn -25.2% deeplist -17.2% retain_wide -15.2% retain1 -15.1% +cycles -14.9% retain -14.7% retain_wide1 -14.3% tree -13.4% +shapes -9.4% tree_wide -6.5% interp -5.9% iso_miss -4.7% +pipeline -3.6% asyncpipe -1.1% fib40 / push_num / churn_read ~0 +``` + +Peak memory footprint is unchanged on every row. + +Two discriminating tests: `u32_keys_take_the_multiplicative_fast_path` goes red +if `write_u32` is deleted (the default forwards to the byte fold and produces a +different digest), and `sequential_shape_ids_spread_across_low_bit_buckets` +goes red if `mix`'s avalanche is dropped, since ShapeIds are a dense run in the +top half of `u32` while `HashMap` indexes on the low bits. + +The symbol profile that found this was previously recorded as structurally +unobtainable. It needs no debug build — only `CARGO_PROFILE_RELEASE_STRIP=none` +plus `PERRY_DEBUG_SYMBOLS=1`, which skips perry's own post-link `strip`. The +"zero `_ZN` frames in `nm`" observation was that `strip`, not LTO +internalization. + +Refs #8125, #8113, #8122. From 83ebcf83fc3047725b0ca528d4f4709957dd6b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 14:04:09 +0200 Subject: [PATCH 3/3] style(fast_hash): snake_case the new test locals clippy --all-targets flagged `viaHash`/`viaU64` as non_snake_case, which the Warnings CI job would surface. No behavioural change. --- crates/perry-runtime/src/fast_hash.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/perry-runtime/src/fast_hash.rs b/crates/perry-runtime/src/fast_hash.rs index bff7e1b82b..9962c062ed 100644 --- a/crates/perry-runtime/src/fast_hash.rs +++ b/crates/perry-runtime/src/fast_hash.rs @@ -204,13 +204,13 @@ mod tests { fn u32_keys_take_the_multiplicative_fast_path() { use std::hash::Hash; for n in [0u32, 1, 0x8000_0000, 0x8000_02ff, u32::MAX] { - let mut viaHash = PtrHasher.build_hasher(); - n.hash(&mut viaHash); - let mut viaU64 = PtrHasher.build_hasher(); - viaU64.write_u64(n as u64); + let mut via_hash = PtrHasher.build_hasher(); + n.hash(&mut via_hash); + let mut via_u64 = PtrHasher.build_hasher(); + via_u64.write_u64(n as u64); assert_eq!( - viaHash.finish(), - viaU64.finish(), + via_hash.finish(), + via_u64.finish(), "u32 key {n:#x} fell into the byte-stream fallback" ); }