From 5d96c8210c0e90c4fbf2e1bd515ac131f3c80bd0 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 2 Nov 2026 18:20:13 +0100 Subject: [PATCH] Use a per-process hash for the dynamic set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Background Interning a string goes through `impl From> for Atom` which does: 1. If the string is short enough, make an inline atom 2. Otherwise if the string is part of the static set, make a static atom 3. Otherwise insert the string in the dynamic set and make a dynamic atom Step 2 involves hashing the string with a keyed hash function whose key is chosen at compile-time by the `phf` crate. phf key search uses a PRNG but with a fixed seed, so the key is deterministic for a given static set. Step 3 involves a separate hash set. To save CPU cycles, string-cache v0.11.0 and earlier re-use the hash already computed from step 2. The dynamic hash set is a custom implementation with a fixed size and no open addressing, so a 12-bit hash collision is enough to put multiple strings in the same bucket and hit the map’s worst case linear time insertion. As reported in https://github.com/servo/string-cache/issues/314, an HTML document adversarially crafted to have many tag or attribute names with hash collisions in a given version of Servo could take quadratic time to parse in that version, just for string interning. ## Mitigation This commit changes the dynamic set to not reuse the hash from the static set, but instead compute its own hash with a per-process instance the standard library’s default hasher `std::hash::RandomState`, another keyed hash function that picks random keys when initialized. The worst case for dynamic atom interning is still linear time to the number of items in the same bucket, but with a random hash key collisions are harder to craft. ## Benchmarking disclaimer All benchmarks results mentioned here are from a laptop. While benches were running the CPU temperature varied between 60°C and 99°C and fan speed beetween off and maximum, so it’s likely that the CPU speed was thermal-throttled some but not all of the time. Precise numbers should be considered unreliable but hopefully we can still discern broad trends. Still it’d be nice if someone wants to reproduce on hardware with more stable clock speed. ## Micro-benchmarks Filtering micro-benchmarks in this repository to those that do string interning: ``` git switch main cargo bench -p integration_tests --bench micro -- --save-baseline 2026-11-03 intern git switch dynamic-hash # this commit cargo bench -p integration_tests --bench micro -- --baseline 2026-11-03 intern ```
Full criterion output ``` static_atom intern time: [11.537 ns 11.677 ns 11.857 ns] change: [−2.5985% −1.7356% −0.5713%] (p = 0.00 < 0.05) Change within noise threshold. Found 4 outliers among 100 measurements (4.00%) 4 (4.00%) high severe short_inline_atom intern time: [6.2984 ns 6.3004 ns 6.3027 ns] change: [+5.5323% +5.7550% +5.9245%] (p = 0.00 < 0.05) Performance has regressed. Found 8 outliers among 100 measurements (8.00%) 2 (2.00%) high mild 6 (6.00%) high severe medium_inline_atom intern time: [2.1287 ns 2.1320 ns 2.1357 ns] change: [+1.4034% +1.8931% +2.3395%] (p = 0.00 < 0.05) Performance has regressed. Found 8 outliers among 100 measurements (8.00%) 6 (6.00%) high mild 2 (2.00%) high severe min_dynamic_atom intern time: [38.216 ns 38.245 ns 38.279 ns] change: [+17.366% +17.463% +17.560%] (p = 0.00 < 0.05) Performance has regressed. Found 4 outliers among 100 measurements (4.00%) 3 (3.00%) high mild 1 (1.00%) high severe longer_dynamic_atom intern time: [49.913 ns 50.593 ns 51.457 ns] change: [+18.758% +20.419% +22.943%] (p = 0.00 < 0.05) Performance has regressed. Found 4 outliers among 100 measurements (4.00%) 4 (4.00%) high severe static_at_runtime intern time: [11.546 ns 11.555 ns 11.567 ns] change: [−1.2172% −1.1122% −1.0048%] (p = 0.00 < 0.05) Performance has improved. Found 12 outliers among 100 measurements (12.00%) 9 (9.00%) high mild 3 (3.00%) high severe intern rand 008 time: [60.782 ns 60.815 ns 60.851 ns] change: [+11.075% +11.158% +11.241%] (p = 0.00 < 0.05) Performance has regressed. intern rand 032 time: [62.676 ns 63.448 ns 64.446 ns] change: [+4.9985% +5.8914% +7.2063%] (p = 0.00 < 0.05) Performance has regressed. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe intern rand 128 time: [92.238 ns 93.480 ns 95.054 ns] change: [+18.802% +20.677% +23.672%] (p = 0.00 < 0.05) Performance has regressed. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe intern rand 512 time: [229.45 ns 229.58 ns 229.74 ns] change: [+41.554% +41.679% +41.801%] (p = 0.00 < 0.05) Performance has regressed. Found 9 outliers among 100 measurements (9.00%) 5 (5.00%) high mild 4 (4.00%) high severe ```
Some number differences for static and inline atoms are deemed statistically significant by Criterion but since no relevant source code has changed I’m attributing this to thermal throttling. As expected interning dynamic atoms is now slower, from +11% time for 8 byte strings to +41% time for 512 bytes. However this is doing nothing but string interning. ## HTML parsing benchmarks On https://github.com/servo/html5ever/pull/813, which adds an HTML input file with all tag and attribute named mofidied to be dynamic atoms. ``` # with string-cache from crates.io cargo bench -p markup5ever_rcdom --bench deserialize-serialize -- --save-baseline 2026-11-03 servo $EDITOR Cargo.toml # use patched string-cache, see below cargo bench -p markup5ever_rcdom --bench deserialize-serialize -- --baseline 2026-11-03 servo ``` To use a patched string-cache, add to html5ever’s root `Cargo.toml`: ```toml [patch.crates-io] string_cache = {path = "../string-cache"} # string-cache on this commit ```
Full criterion output ``` deserialize "servo.org.html" time: [225.43 µs 225.55 µs 225.67 µs] change: [+1.7193% +1.8193% +1.9216%] (p = 0.00 < 0.05) Performance has regressed. Found 4 outliers among 100 measurements (4.00%) 1 (1.00%) low mild 2 (2.00%) high mild 1 (1.00%) high severe Benchmarking deserialize "servo.org-custom.html": Collecting 100 samples in esdeserialize "servo.org-custom.html" time: [302.33 µs 302.72 µs 303.08 µs] change: [+0.4744% +0.6366% +0.7950%] (p = 0.00 < 0.05) Change within noise threshold. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) low mild Benchmarking serialize "servo.org.html": Collecting 100 samples in estimated 5serialize "servo.org.html" time: [18.591 µs 18.643 µs 18.715 µs] change: [−1.0722% +0.8353% +2.7065%] (p = 0.39 > 0.05) No change in performance detected. Found 13 outliers among 100 measurements (13.00%) 10 (10.00%) low severe 3 (3.00%) high severe Benchmarking serialize "servo.org-custom.html": Collecting 100 samples in estiserialize "servo.org-custom.html" time: [27.822 µs 27.858 µs 27.890 µs] change: [−4.6615% −3.6017% −2.4905%] (p = 0.00 < 0.05) Performance has improved. Found 16 outliers among 100 measurements (16.00%) 16 (16.00%) low severe ```
String interning only happens during HTML deserialize, and `servo.org.html` contains ~no dynamic atoms so only `deserialize "servo.org-custom.html"` is relevant here. Once again I attribute other changes to thermal throttling. `deserialize "servo.org-custom.html"` shows not siginificant change, which I believe indicates that the performance regression on string interning is small when part of HTML parsing. Signed-off-by: Simon Sapin --- src/atom.rs | 16 +++++----------- src/dynamic_set.rs | 13 +++++++++++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/atom.rs b/src/atom.rs index 31dafd2..a43f584 100644 --- a/src/atom.rs +++ b/src/atom.rs @@ -211,18 +211,14 @@ impl Atom { } pub fn try_static(string_to_add: &str) -> Option { - Self::try_static_internal(string_to_add).ok() - } - - fn try_static_internal(string_to_add: &str) -> Result { let static_set = Static::get(); let hash = phf_shared::hash(string_to_add, &static_set.key); let index = phf_shared::get_index(&hash, static_set.disps, static_set.atoms.len()); if static_set.atoms[index as usize] == string_to_add { - Ok(Self::pack_static(index)) + Some(Self::pack_static(index)) } else { - Err(hash) + None } } @@ -271,11 +267,9 @@ impl<'a, Static: StaticAtomSet> From> for Atom { phantom: PhantomData, } } else { - Self::try_static_internal(&string_to_add).unwrap_or_else(|hash| { - // Reconstitute 64-bit `Hash128::h1` - // https://docs.rs/phf_shared/0.14.0/src/phf_shared/lib.rs.html#45-54 - let hash = (hash.g as u64) << 32 | (hash.f1 as u64); - let ptr: std::ptr::NonNull = dynamic_set().insert(string_to_add, hash); + Self::try_static(&string_to_add).unwrap_or_else(|| { + // This string is not in the static set, add it if needed to the dynamic set. + let ptr: std::ptr::NonNull = dynamic_set().insert(string_to_add); let data = ptr.as_ptr().expose_provenance() as u64; debug_assert!(0 == data & TAG_MASK); Atom { diff --git a/src/dynamic_set.rs b/src/dynamic_set.rs index 19ffb72..fc89859 100644 --- a/src/dynamic_set.rs +++ b/src/dynamic_set.rs @@ -10,6 +10,7 @@ use parking_lot::Mutex; use std::borrow::Cow; use std::cell::UnsafeCell; +use std::hash::{BuildHasher as _, RandomState}; use std::ptr::NonNull; use std::sync::OnceLock; use std::sync::atomic::AtomicIsize; @@ -20,6 +21,7 @@ const BUCKET_MASK: u64 = (1 << 12) - 1; pub(crate) struct Set { buckets: Box<[Mutex>>]>, + hasher: RandomState, } pub(crate) struct Entry { @@ -46,12 +48,19 @@ pub(crate) fn dynamic_set() -> &'static Set { DYNAMIC_SET.get_or_init(|| { let buckets = (0..NB_BUCKETS).map(|_| Mutex::new(None)).collect(); - Set { buckets } + Set { + buckets, + hasher: RandomState::new(), + } }) } impl Set { - pub(crate) fn insert(&self, string: Cow, hash: u64) -> NonNull { + pub(crate) fn insert(&self, string: Cow) -> NonNull { + // Don’t reuse the static set hash here: + // https://github.com/servo/string-cache/issues/314 + let hash = self.hasher.hash_one::<&str>(&string); + let bucket_index = (hash & BUCKET_MASK) as usize; let mut linked_list = self.buckets[bucket_index].lock();