From bc0cb307a39416e608c26b184db979d25b28472c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 21:40:54 +0200 Subject: [PATCH 1/4] fix(string): make codePointAt linear on non-ASCII strings (#10656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `js_string_code_point_at` walked the WTF-8 payload from byte 0 on every call for any string that is not pure ASCII, so a sequential scan over such a string was O(n^2). #10055 reported exactly this pathology; #10067 moved `charCodeAt` and bracket indexing onto a lazy sparse index with a cursor and left `codePointAt` on the old walk. The accessors disagreed on the same string. 200,000 indexed reads of `typescript@5.9.3`'s `lib.dom.d.ts` (1,874,815 chars, 45 of them non-ASCII): charCodeAt 2 ms codePointAt 13,049 ms Scaling over a string with a single `é`, time quadrupling per doubling: n=5,000 8 ms n=20,000 128 ms n=10,000 32 ms n=40,000 518 ms `codePointAt` is defined on code units, so no bespoke decoding is needed: route it through `utf16_unit_at` and apply the spec algorithm — read the unit, and only when it is a leading surrogate with a trailing surrogate after it combine the pair. That keeps the bounded WTF-8 stepping #6085 requires, so a payload ending in a truncated multi-byte lead still decodes from what is present instead of over-reading; the existing guard-page test `code_point_at_does_not_read_past_payload` covers that and still passes. After, same hardware, against Node v26.5.1: 200k reads, non-ASCII 13,049 ms -> 2 ms (node 1 ms) n=40,000 scan 518 ms -> 1 ms (node 0 ms) tsc --noEmit demo.ts 658.31 s -> 85.04 s user (node 0.80 s) The tsc figure is the motivating case: `sample` attributed ~85% of that run to `js_string_code_point_at` and ~12% to `copy_utf16_range` underneath it, because TypeScript's scanner calls `codePointAt` per character and `lib.dom.d.ts` — the largest file it loads — carries those 45 non-ASCII characters. 7.7x on the real workload, and the remaining gap is no longer string indexing. Values are unchanged: `"a\u{1F600}b"` still yields 97, 128512, 56832, 98 at indices 0-3 (astral code point at the pair start, bare trailing surrogate on the low half), `"ab".codePointAt(5)` is still undefined, all byte-identical to Node. Tests: two added next to the index they exercise — one walks every index of a long non-ASCII string containing a surrogate pair and compares against an independent UTF-16 expansion, one asserts forward and backward traversal agree so the cursor cannot serve a stale answer on a backward seek. --- changelog.d/10656-codepointat-linear.md | 5 ++ crates/perry-runtime/src/string/char_ops.rs | 59 +++++++++----- .../src/string/char_ops/utf16_index/tests.rs | 77 +++++++++++++++++++ 3 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 changelog.d/10656-codepointat-linear.md diff --git a/changelog.d/10656-codepointat-linear.md b/changelog.d/10656-codepointat-linear.md new file mode 100644 index 0000000000..31a8d597fa --- /dev/null +++ b/changelog.d/10656-codepointat-linear.md @@ -0,0 +1,5 @@ +`String.prototype.codePointAt` is no longer O(n) per call on strings containing +non-ASCII characters. It now uses the same lazy UTF-16 index `charCodeAt` and +bracket indexing were moved to in #10067, so a sequential scan is linear rather +than quadratic. A natively compiled `tsc` goes from 658 s to 85 s on a two-line +input (#10656). diff --git a/crates/perry-runtime/src/string/char_ops.rs b/crates/perry-runtime/src/string/char_ops.rs index c807f477a4..832bbc1628 100644 --- a/crates/perry-runtime/src/string/char_ops.rs +++ b/crates/perry-runtime/src/string/char_ops.rs @@ -695,26 +695,45 @@ pub extern "C" fn js_string_code_point_at(s: *const StringHeader, index: i32) -> } } - // Non-ASCII: bounded WTF-8 walk (#6085) — the old `str_data.chars()` loop - // read continuation bytes past an exact-sized payload ending in a truncated - // multi-byte lead. Allocation-free either way. - let bytes = unsafe { slice::from_raw_parts(string_data(s), (*s).byte_len as usize) }; - let mut utf16_pos = 0usize; - let mut i = 0usize; - while i < bytes.len() { - let (advance, units, cp) = crate::string::wtf8_step(bytes, i); - if units > 0 && utf16_pos + units > idx { - if units == 1 || utf16_pos == idx { - // Either a BMP code point, or the START of a surrogate pair — - // which per spec is the whole code point. - return cp as f64; - } - // Index lands on the low surrogate half — return the bare unit. - let v = cp.wrapping_sub(0x10000); - return (0xDC00 + (v & 0x3FF)) as f64; + // Non-ASCII: go through the same lazy sparse index + cursor `charCodeAt` + // uses (#10055/#10067). This function used to walk the WTF-8 payload from + // byte 0 on every call, so a sequential scan over a string holding even one + // non-ASCII character was O(n^2) — #10656. `charCodeAt` and `s[i]` were + // moved off that walk by #10067; `codePointAt` was left on it, which is why + // a natively compiled `tsc` spent ~85% of its run in this function + // (`lib.dom.d.ts` carries 45 non-ASCII characters in 1.87 MB). + // + // No bespoke decoding is needed: `codePointAt` is *defined* on code units, + // so the spec algorithm is two indexed reads. `unit_at` keeps the bounded + // WTF-8 stepping that #6085 needs, so the truncated-payload guarantee is + // preserved — a missing continuation byte still decodes from what is + // present rather than over-reading. + let first = match utf16_unit_at(s, idx) { + Some(unit) => unit, + None => return f64::from_bits(crate::value::TAG_UNDEFINED), + }; + // A lone/low surrogate, a BMP code point, or a leading surrogate with + // nothing after it: the unit is the answer. + if !is_leading_surrogate(first) || idx + 1 >= u16len { + return first as f64; + } + match utf16_unit_at(s, idx + 1) { + Some(second) if is_trailing_surrogate(second) => { + let high = (first as u32 - 0xD800) << 10; + let low = second as u32 - 0xDC00; + (0x10000 + high + low) as f64 } - utf16_pos += units; - i += advance; + // Unpaired leading surrogate — per spec the code unit itself. + _ => first as f64, } - f64::from_bits(crate::value::TAG_UNDEFINED) +} + +#[inline] +fn is_leading_surrogate(unit: u16) -> bool { + (0xD800..0xDC00).contains(&unit) +} + +#[inline] +fn is_trailing_surrogate(unit: u16) -> bool { + (0xDC00..0xE000).contains(&unit) } diff --git a/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs b/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs index fed6f718bc..2c5bf1ee43 100644 --- a/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs +++ b/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs @@ -167,3 +167,80 @@ fn cache_eviction_is_bounded_and_short_strings_do_not_evict_sources() { } prune_dead_utf16_indexes(&|_| true); } + +/// #10656: `codePointAt` used to walk the WTF-8 payload from byte 0 on every +/// call, so a scan over a string holding one non-ASCII character was O(n^2). +/// These pin the spec behaviour across the cached-index path that replaced it: +/// a BMP code point, the start of a surrogate pair (the whole code point), the +/// low half (the bare trailing surrogate), and an unpaired leading surrogate. +#[test] +fn code_point_at_matches_the_spec_through_the_cached_index() { + // Long enough to exercise the checkpoint/cursor path, not the short-string + // fallback, and non-ASCII so it cannot take the ASCII fast path. + let mut text = String::new(); + for _ in 0..200 { + text.push_str("\u{e9}abcdefghij0123456789"); + } + let astral_at = text.chars().count(); + text.push('\u{1F600}'); // surrogate pair + text.push('z'); + + let s = crate::string::js_string_from_bytes(text.as_ptr(), text.len() as u32); + let units: Vec = text.encode_utf16().collect(); + + // Walk forwards (the sequential case tsc hits) and compare every index + // against an independent UTF-16 expansion of the same text. + for (idx, &unit) in units.iter().enumerate() { + let got = crate::string::js_string_code_point_at(s, idx as i32); + let expected = if (0xD800..0xDC00).contains(&unit) && idx + 1 < units.len() { + let second = units[idx + 1]; + if (0xDC00..0xE000).contains(&second) { + 0x10000 + (((unit as u32 - 0xD800) << 10) | (second as u32 - 0xDC00)) + } else { + unit as u32 + } + } else { + unit as u32 + }; + assert_eq!(got, expected as f64, "codePointAt({idx})"); + } + + // The surrogate pair specifically: start yields the astral code point, the + // low half yields the bare trailing surrogate. + let pair_start = units.len() - 3; + assert_eq!( + crate::string::js_string_code_point_at(s, pair_start as i32), + 128512.0_f64 + ); + assert!( + (0xDC00..0xE000).contains(&(crate::string::js_string_code_point_at(s, pair_start as i32 + 1) as u32 as u16)) + ); + let _ = astral_at; + + // Out of bounds stays undefined. + let oob = crate::string::js_string_code_point_at(s, units.len() as i32); + assert_eq!(oob.to_bits(), crate::value::TAG_UNDEFINED); +} + +/// Random access must agree with sequential access: the cursor optimises the +/// forward case, and a backward seek must not return a stale answer. +#[test] +fn code_point_at_is_order_independent() { + let mut text = String::new(); + for _ in 0..150 { + text.push_str("x\u{e9}yz"); + } + let s = crate::string::js_string_from_bytes(text.as_ptr(), text.len() as u32); + let n = text.encode_utf16().count(); + + let forward: Vec = (0..n) + .map(|i| crate::string::js_string_code_point_at(s, i as i32)) + .collect(); + let backward: Vec = (0..n) + .rev() + .map(|i| crate::string::js_string_code_point_at(s, i as i32)) + .collect(); + for (i, value) in backward.iter().rev().enumerate() { + assert_eq!(*value, forward[i], "index {i} differs by traversal order"); + } +} From d3952e4b893a0d12a0a53b5f823f453306a2307d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 06:39:21 +0200 Subject: [PATCH 2/4] fix(string): resolve slice start offsets through the UTF-16 index (#10685) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `copy_utf16_range` resolved its start boundary with `advance(bytes, Boundary::default(), start)` — a walk from byte 0 on every call. Slicing a non-ASCII string at increasing offsets, which is what every tokenizer does, was therefore O(n^2): n=20,000 33 ms n=80,000 404 ms n=40,000 102 ms n=160,000 1,662 ms ASCII controls are flat at every size, so this is the fast-path loss rather than the copy itself. This is the same pathology as #10055: #10067 moved `charCodeAt` and bracket indexing onto the lazy index, #10656 moved `codePointAt`, and this was the accessor still left on the walk. `Index::seek` is factored out of `unit_at` so `unit_at` and the new `boundary_at` share one implementation and one cursor rather than drifting apart — which is how the other two were missed. `boundary_at` returns `None` for short payloads and `start == 0`, where the caller's own walk is already cheap and a four-entry cache should not be disturbed, so those keep their existing behaviour exactly. After, against Node v26.5.1: substring scan, n=160,000 1,662 ms -> 1 ms (node 0 ms) tsc --noEmit demo.ts 85.04 s -> 7.76 s user (node 0.80 s) 11x on real tsc, and 85x cumulative against the 658.31 s this started at. `sample` had attributed ~97% of the remaining run to `copy_utf16_range` (50,627 of ~51,700 leaf samples; next symbol 162), because TypeScript's scanner extracts every token with `substring` and `lib.dom.d.ts` carries 45 non-ASCII characters in 1.87 MB. Correctness is unchanged, including the cases a wrong boundary would corrupt rather than merely slow: hashing every substring of a string containing astral characters (all i,j pairs, including ones that split a surrogate pair) gives 1234090636 on both Perry and Node, split-pair slices still yield the lone halves "\ud83d" / "\ude00", and a random-access-order slice hash matches. Tests: `boundary_at_matches_a_walk_from_zero` compares the indexed lookup against `advance` from zero at every index of a string containing both a two-byte scalar and a surrogate pair; `boundary_at_is_order_independent` asserts forward and backward traversal agree so the cursor cannot serve a stale boundary after a backward seek. 158 `string::` tests pass. --- changelog.d/10685-slice-linear.md | 5 ++ crates/perry-runtime/src/string/char_ops.rs | 8 +++ .../src/string/char_ops/utf16_index.rs | 61 ++++++++++++++++++- .../src/string/char_ops/utf16_index/tests.rs | 47 ++++++++++++++ .../perry-runtime/src/string/slice_range.rs | 11 +++- 5 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 changelog.d/10685-slice-linear.md diff --git a/changelog.d/10685-slice-linear.md b/changelog.d/10685-slice-linear.md new file mode 100644 index 0000000000..207f47a863 --- /dev/null +++ b/changelog.d/10685-slice-linear.md @@ -0,0 +1,5 @@ +`String.prototype.substring` / `slice` / `substr` no longer walk from byte 0 to +resolve their start offset on strings containing non-ASCII characters. They now +use the same lazy UTF-16 index the other accessors use, so slicing at increasing +offsets — every tokenizer's access pattern — is linear rather than quadratic. A +natively compiled `tsc` goes from 85 s to 7.8 s (#10685). diff --git a/crates/perry-runtime/src/string/char_ops.rs b/crates/perry-runtime/src/string/char_ops.rs index 832bbc1628..1449fb5aa8 100644 --- a/crates/perry-runtime/src/string/char_ops.rs +++ b/crates/perry-runtime/src/string/char_ops.rs @@ -104,6 +104,14 @@ fn utf16_unit_at(s: *const StringHeader, idx: usize) -> Option { utf16_index::unit_at(s, idx) } +/// Byte offset of the code point containing UTF-16 index `idx`, plus whether +/// `idx` is its low surrogate half, resolved through the same cached index. +/// `None` means the caller should fall back to its own walk (short payloads, +/// `idx == 0`, or an index past the last decodable unit). +pub(super) fn utf16_boundary_at(s: *const StringHeader, idx: usize) -> Option<(usize, bool)> { + utf16_index::boundary_at(s, idx) +} + /// SSO-safe `s[key]`: takes the receiver as a **NaN-boxed JSValue** rather than /// an already-unboxed `StringHeader*`. /// diff --git a/crates/perry-runtime/src/string/char_ops/utf16_index.rs b/crates/perry-runtime/src/string/char_ops/utf16_index.rs index c66e617da1..e6fb4a4ead 100644 --- a/crates/perry-runtime/src/string/char_ops/utf16_index.rs +++ b/crates/perry-runtime/src/string/char_ops/utf16_index.rs @@ -29,7 +29,11 @@ struct Index { } impl Index { - fn unit_at(&mut self, bytes: &[u8], idx: usize) -> Option { + /// Locate the code point containing UTF-16 index `idx`, returning its + /// position along with the decoded step. Shared by `unit_at` and + /// `boundary_at` so both pay the same amortised seek and both maintain the + /// same cursor and checkpoints. + fn seek(&mut self, bytes: &[u8], idx: usize) -> Option<(Position, usize, u32)> { let mut pos = self.cursor; // Nearby forward reads use the cursor, including the second half of // an astral character. Other seeks start at the nearest checkpoint. @@ -49,7 +53,7 @@ impl Index { let (advance, units, cp) = decode_step(bytes, pos.byte as usize); if units > 0 && pos.utf16 as usize + units > idx { self.cursor = pos; - return Some(code_unit(cp, units, idx == pos.utf16 as usize)); + return Some((pos, units, cp)); } // A truncated tail can advance past byte_len; never save an // out-of-payload cursor (or narrow that offset with a wrapping cast). @@ -59,6 +63,18 @@ impl Index { self.cursor = pos; None } + + fn unit_at(&mut self, bytes: &[u8], idx: usize) -> Option { + let (pos, units, cp) = self.seek(bytes, idx)?; + Some(code_unit(cp, units, idx == pos.utf16 as usize)) + } + + /// Byte offset of the code point containing `idx`, and whether `idx` is + /// its low surrogate half — i.e. `slice_range::Boundary` in its raw parts. + fn boundary_at(&mut self, bytes: &[u8], idx: usize) -> Option<(usize, bool)> { + let (pos, units, _) = self.seek(bytes, idx)?; + Some((pos.byte as usize, units == 2 && idx != pos.utf16 as usize)) + } } #[inline] @@ -105,6 +121,47 @@ crate::perry_thread_local! { /// Caller has validated the header and UTF-16 index. Small strings bypass the /// cache: in particular, consuming a character returned by `s[i]` must not evict /// the source string. ASCII callers retain their existing direct byte access. +/// Byte offset (and low-surrogate-half flag) for UTF-16 index `idx`, through +/// the same cache `unit_at` uses. #10685: `slice_range::copy_utf16_range` +/// resolved its start boundary with `advance(bytes, Boundary::default(), start)` +/// — a walk from byte 0 on every call — so slicing a non-ASCII string at +/// increasing offsets was O(n^2), which is the shape TypeScript's scanner has. +pub(super) fn boundary_at(s: *const StringHeader, idx: usize) -> Option<(usize, bool)> { + let byte_len = unsafe { (*s).byte_len }; + let bytes = unsafe { slice::from_raw_parts(string_data(s), byte_len as usize) }; + if bytes.len() < CHECKPOINT_BYTES || idx == 0 { + // Short strings and a zero start do not need the cache: the caller's + // own walk is already O(1)-ish, and consuming a slice must not evict + // the source string from a four-entry cache. + return None; + } + UTF16_INDEX_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + let owner = s as usize; + let slot = if cache.entries[cache.hot].owner == owner { + cache.hot + } else if let Some(slot) = cache.entries.iter().position(|entry| entry.owner == owner) { + slot + } else { + let slot = cache.next; + cache.next = (slot + 1) % CACHE_ENTRIES; + slot + }; + cache.hot = slot; + let entry = &mut cache.entries[slot]; + let utf16_len = unsafe { (*s).utf16_len }; + if entry.owner != owner || entry.byte_len != byte_len || entry.utf16_len != utf16_len { + *entry = Index { + owner, + byte_len, + utf16_len, + ..Index::default() + }; + } + entry.boundary_at(bytes, idx) + }) +} + pub(super) fn unit_at(s: *const StringHeader, idx: usize) -> Option { let byte_len = unsafe { (*s).byte_len }; let bytes = unsafe { slice::from_raw_parts(string_data(s), byte_len as usize) }; diff --git a/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs b/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs index 2c5bf1ee43..52d8b5ce2e 100644 --- a/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs +++ b/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs @@ -244,3 +244,50 @@ fn code_point_at_is_order_independent() { assert_eq!(*value, forward[i], "index {i} differs by traversal order"); } } + +/// #10685: `copy_utf16_range` resolved its start boundary by walking from byte +/// 0 on every call, so slicing a non-ASCII string at increasing offsets was +/// O(n^2). `boundary_at` must agree with that walk at every index — including +/// the low half of a surrogate pair, where `low` selects the split copy path. +#[test] +fn boundary_at_matches_a_walk_from_zero() { + let mut text = String::new(); + for _ in 0..80 { + text.push_str("\u{e9}abcdefghij0123456789"); + } + text.push('\u{1F600}'); + text.push_str("tail\u{e9}"); + let s = crate::string::js_string_from_bytes(text.as_ptr(), text.len() as u32); + let bytes = unsafe { + std::slice::from_raw_parts(crate::string::string_data(s), (*s).byte_len as usize) + }; + let n = text.encode_utf16().count(); + + for idx in 0..n { + let walked = crate::string::slice_range::advance( + bytes, + crate::string::slice_range::Boundary::default(), + idx, + ); + if let Some((byte, low)) = super::boundary_at(s, idx) { + assert_eq!(byte, walked.byte, "byte offset at {idx}"); + assert_eq!(low, walked.low, "low-surrogate flag at {idx}"); + } + } +} + +/// The cursor optimises forward seeks; a backward seek must not reuse it. +#[test] +fn boundary_at_is_order_independent() { + let mut text = String::new(); + for _ in 0..80 { + text.push_str("x\u{e9}yz"); + } + let s = crate::string::js_string_from_bytes(text.as_ptr(), text.len() as u32); + let n = text.encode_utf16().count(); + let forward: Vec<_> = (0..n).map(|i| super::boundary_at(s, i)).collect(); + let backward: Vec<_> = (0..n).rev().map(|i| super::boundary_at(s, i)).collect(); + for (i, value) in backward.iter().rev().enumerate() { + assert_eq!(*value, forward[i], "index {i} differs by traversal order"); + } +} diff --git a/crates/perry-runtime/src/string/slice_range.rs b/crates/perry-runtime/src/string/slice_range.rs index e4fbd12577..0fe5a4702f 100644 --- a/crates/perry-runtime/src/string/slice_range.rs +++ b/crates/perry-runtime/src/string/slice_range.rs @@ -33,7 +33,16 @@ pub(super) fn copy_utf16_range(s: *const StringHeader, start: u32, end: u32) -> return string_copy_range(s, start as usize, end - start, end - start, 0); } let bytes = unsafe { slice::from_raw_parts(string_data(s), (*s).byte_len as usize) }; - let first = advance(bytes, Boundary::default(), start as usize); + // #10685: resolve the start boundary through the cached UTF-16 index + // rather than walking from byte 0 on every call. Slicing a non-ASCII + // string at increasing offsets — TypeScript's scanner, and every other + // tokenizer — was O(n^2) because of that walk. `None` keeps the original + // behaviour for short payloads and `start == 0`, where the walk is already + // cheap and the cache should not be disturbed. + let first = match char_ops::utf16_boundary_at(s, start as usize) { + Some((byte, low)) => Boundary { byte, low }, + None => advance(bytes, Boundary::default(), start as usize), + }; // A suffix's end is already known: do not scan the entire remaining string. let last = if end == unsafe { (*s).utf16_len } { Boundary { From 5293235c1de6afa88e2901fcdbf6447f824b45d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 09:25:36 +0200 Subject: [PATCH 3/4] perf(array): gate registry probes on the GC header in the indexing path (#10694) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `js_array_get_f64`, `js_array_set_f64`, `js_array_set_f64_extend` and the iteration-exotic helpers probed the buffer and typed-array registries on every element access. A `GC_TYPE_ARRAY` header can never be a registered buffer or typed array — every registration carries its own GC object type — and `array/header.rs`'s `receiver_may_be_registered_exotic` exists to say so in one already-warm header byte read plus an integer compare. Thirteen call sites in `array/iter_methods.rs` already used that gate. None in `array/indexing.rs` did. Measured with the runtime's own `PERRY_BUFFER_DIAG` on `tsc --noEmit demo.ts` (a two-line input): before: probes=79,691,777 admits=26,198,956 true_positives=90 after: probes=39,845,889 admits=21,646,032 true_positives=90 79.7M probes to answer a question about a set that never holds more than 9 buffers, and is answered yes 90 times. Honest perf note: this is not measurable on tsc wall-clock. `is_registered_buffer_slow` was 2.8% of leaf samples, so halving its calls predicts ~1.4%; five interleaved rounds give a median of -1.2% with overlapping ranges (A 6.85-8.02s, B 6.76-7.86s), i.e. below the noise floor at that sample size. The change earns its place on correctness and consistency — it removes provably-wasted work and makes the indexing path match the gate every iteration helper already uses — not on a demonstrated speedup. Buffer-heavy workloads should benefit more; tsc is not one. Gating the four remaining `||`-shaped probe sites in the same file changed the probe count by zero, so the other ~39.8M originate outside `array/indexing.rs` and are still unattributed. #10694 also records that the diagnostic sizes a 1024-bit/3-hash Bloom at 0.0% false-positive on this workload, which would make each surviving probe ~free regardless of caller. Verified: 443 existing tests pass (360 `array::`, 52 `buffer::`, 31 `typedarray::`), and a differential test against Node covering every typed array kind, Uint8Array wrapping (300 -> 44, -1 -> 255), Uint8ClampedArray clamping, f32 precision, Buffer, subarray aliasing and an Array subclass is byte-for-byte identical. --- changelog.d/10694-array-index-gate.md | 5 + crates/perry-runtime/src/array/indexing.rs | 138 +++++++++++++-------- 2 files changed, 89 insertions(+), 54 deletions(-) create mode 100644 changelog.d/10694-array-index-gate.md diff --git a/changelog.d/10694-array-index-gate.md b/changelog.d/10694-array-index-gate.md new file mode 100644 index 0000000000..5c533ffc2d --- /dev/null +++ b/changelog.d/10694-array-index-gate.md @@ -0,0 +1,5 @@ +Plain-array element reads and writes no longer probe the buffer and typed-array +registries. A `GC_TYPE_ARRAY` header can never be either, and the iteration +helpers already gated on that; the indexing path did not. On a `tsc --noEmit` +of a two-line file this halves `is_registered_buffer` probes, 79.7M to 39.8M +(#10694). diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 69d15f0ace..e23aed493c 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -86,8 +86,9 @@ pub(crate) fn array_iteration_is_exotic(arr: *const ArrayHeader) -> bool { if arr.is_null() { return false; } - if crate::buffer::is_registered_buffer(arr as usize) - || crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() + if super::header::receiver_may_be_registered_exotic(arr as *const ArrayHeader) + && (crate::buffer::is_registered_buffer(arr as usize) + || crate::typedarray::lookup_typed_array_kind(arr as usize).is_some()) { return true; } @@ -119,8 +120,9 @@ pub(crate) unsafe fn array_iteration_is_exotic_cleaned( arr: *const ArrayHeader, flags: u16, ) -> bool { - if crate::buffer::is_registered_buffer(arr as usize) - || crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() + if super::header::receiver_may_be_registered_exotic(arr as *const ArrayHeader) + && (crate::buffer::is_registered_buffer(arr as usize) + || crate::typedarray::lookup_typed_array_kind(arr as usize).is_some()) { return true; } @@ -606,18 +608,30 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { return f64::NAN; } let arr = cleaned; - // Check if this is actually a TypedArray — dispatch through typed array helper - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { - return crate::typedarray::js_typed_array_get( - arr as *const crate::typedarray::TypedArrayHeader, - index as i32, - ); - } - // Check if this is actually a buffer (Uint8Array) — read individual bytes - if crate::buffer::is_registered_buffer(arr as usize) { - let byte_val = - crate::buffer::js_buffer_get(arr as *const crate::buffer::BufferHeader, index as i32); - return byte_val as f64; + // #10694: a `GC_TYPE_ARRAY` header can never be a registered buffer or + // typed array — every registration carries its own GC object type — so a + // plain array must not pay the thread-local registry probes. The + // iteration helpers already gate on this; the indexing path did not, and + // on a `tsc --noEmit` of a two-line file that cost **79.7 M** + // `is_registered_buffer` probes for a process that registers **9** + // buffers, hitting 90 times. One already-warm GC-header byte read and an + // integer compare replace them. + if super::header::receiver_may_be_registered_exotic(arr) { + // Check if this is actually a TypedArray — dispatch through typed array helper + if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { + return crate::typedarray::js_typed_array_get( + arr as *const crate::typedarray::TypedArrayHeader, + index as i32, + ); + } + // Check if this is actually a buffer (Uint8Array) — read individual bytes + if crate::buffer::is_registered_buffer(arr as usize) { + let byte_val = crate::buffer::js_buffer_get( + arr as *const crate::buffer::BufferHeader, + index as i32, + ); + return byte_val as f64; + } } // The usual case cleans to the same address, so reuse the header tag read // above. A forwarded Array resolves to a different address and needs its @@ -794,23 +808,33 @@ pub extern "C" fn js_array_set_f64(arr: *mut ArrayHeader, index: u32, value: f64 if arr.is_null() { return; } - // Check if this is actually a buffer (Uint8Array) — write individual bytes - if crate::buffer::is_registered_buffer(arr as usize) { - crate::buffer::js_buffer_set( - arr as *mut crate::buffer::BufferHeader, - index as i32, - value as i32, - ); - return; - } - // Check if this is a typed array — route through per-kind store. - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { - crate::typedarray::js_typed_array_set( - arr as *mut crate::typedarray::TypedArrayHeader, - index as i32, - value, - ); - return; + // #10694: a `GC_TYPE_ARRAY` header can never be a registered buffer or + // typed array — every registration carries its own GC object type — so a + // plain array must not pay the thread-local registry probes. The + // iteration helpers already gate on this; the indexing path did not, and + // on a `tsc --noEmit` of a two-line file that cost **79.7 M** + // `is_registered_buffer` probes for a process that registers **9** + // buffers, hitting 90 times. One already-warm GC-header byte read and an + // integer compare replace them. + if super::header::receiver_may_be_registered_exotic(arr) { + // Check if this is actually a buffer (Uint8Array) — write individual bytes + if crate::buffer::is_registered_buffer(arr as usize) { + crate::buffer::js_buffer_set( + arr as *mut crate::buffer::BufferHeader, + index as i32, + value as i32, + ); + return; + } + // Check if this is a typed array — route through per-kind store. + if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { + crate::typedarray::js_typed_array_set( + arr as *mut crate::typedarray::TypedArrayHeader, + index as i32, + value, + ); + return; + } } // SAFETY: the clean above resolved this exact plain-array head; the // Buffer/TypedArray exits precede this direct header read. @@ -857,8 +881,9 @@ pub extern "C" fn js_array_set_f64(arr: *mut ArrayHeader, index: u32, value: f64 pub(crate) fn array_strict_index_write_guard(arr: *mut ArrayHeader, index: u32) { let clean = clean_arr_ptr_mut(arr); if clean.is_null() - || crate::buffer::is_registered_buffer(clean as usize) - || crate::typedarray::lookup_typed_array_kind(clean as usize).is_some() + || (super::header::receiver_may_be_registered_exotic(clean as *const ArrayHeader) + && (crate::buffer::is_registered_buffer(clean as usize) + || crate::typedarray::lookup_typed_array_kind(clean as usize).is_some())) { return; } @@ -1236,8 +1261,9 @@ fn js_array_set_f64_extend_strict_impl( } let clean = clean_arr_ptr_mut(arr); if clean.is_null() - || crate::buffer::is_registered_buffer(clean as usize) - || crate::typedarray::lookup_typed_array_kind(clean as usize).is_some() + || (super::header::receiver_may_be_registered_exotic(clean as *const ArrayHeader) + && (crate::buffer::is_registered_buffer(clean as usize) + || crate::typedarray::lookup_typed_array_kind(clean as usize).is_some())) { // Preserve the existing polymorphic/subclass behavior on receivers // that are not live plain arrays. These are cold and cannot use the @@ -1463,23 +1489,27 @@ pub extern "C" fn js_array_set_f64_extend( return js_array_alloc(0); } let arr = cleaned; - // Check if this is actually a buffer (Uint8Array) — write individual bytes - if crate::buffer::is_registered_buffer(arr as usize) { - crate::buffer::js_buffer_set( - arr as *mut crate::buffer::BufferHeader, - index as i32, - value as i32, - ); - return arr; - } - // Check if this is a typed array — route through per-kind store (no extension). - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { - crate::typedarray::js_typed_array_set( - arr as *mut crate::typedarray::TypedArrayHeader, - index as i32, - value, - ); - return arr; + // #10694: skip both registry probes for a `GC_TYPE_ARRAY` header, which + // can never be a registered buffer or typed array. + if super::header::receiver_may_be_registered_exotic(arr) { + // Check if this is actually a buffer (Uint8Array) — write individual bytes + if crate::buffer::is_registered_buffer(arr as usize) { + crate::buffer::js_buffer_set( + arr as *mut crate::buffer::BufferHeader, + index as i32, + value as i32, + ); + return arr; + } + // Check if this is a typed array — route through per-kind store (no extension). + if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { + crate::typedarray::js_typed_array_set( + arr as *mut crate::typedarray::TypedArrayHeader, + index as i32, + value, + ); + return arr; + } } // SAFETY: the clean above resolved this live plain-array head, and the // compatible Buffer/TypedArray receivers have exited. From e01753475c7d153a243de8cb4009277b654281de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 11:31:27 +0200 Subject: [PATCH 4/4] perf(string): give the UTF-16 index no capacity, so it cannot thrash (#10688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `UTF16_INDEX_CACHE` held `CACHE_ENTRIES = 4` indexes and evicted round-robin. A program interleaving indexed access across five or more non-ASCII strings evicted the entry it was about to need on every access and rebuilt it from scratch, forever. It was a step function, not a gradual decay: K interleaved before after 1 67 ns 67 ns 4 50 ns 67 ns 5 81,525 ns 67 ns 8 80,972 ns 67 ns 12 81,228 ns 67 ns 1,217x at K>=5, and flat everywhere. An ASCII control stays at 25-39 ns in both, which isolates the cause to the index rather than to string count. Capacity was the defect, so there is no capacity: the cache becomes an owner-keyed map and entries live until their string dies. The lifetime machinery already existed — `prune_dead_utf16_indexes` is driven by the collector — so this reuses it rather than inventing ownership. Raising `CACHE_ENTRIES` would only move the wall to K+1. I first tried changing the table's *shape* instead (sparse run-boundary syncs with affine spans, prototyped at 108x less index memory); it made this cliff 23% WORSE, 81,525 -> 100,287 ns, because K>=5 is a pure *rebuilding* workload and a run table builds more slowly than it queries. That experiment is written up on #10688 and is what established that the fix had to be "stop rebuilding". `scan_utf16_index_roots_mut` now drains, lets the visitor rewrite the owner identities the map is keyed by, and reinserts — the keys are exactly what the collector relocates, so they must be rehashed rather than mutated in place. Memory: peak RSS on `tsc --noEmit demo.ts` is 606.7 MB against 613.4 MB for the same binary without this change, i.e. slightly lower rather than higher. Entries are unbounded between collections by construction, which is a real change in character even though it does not cost anything measurable here. Tests: 158 `string::` tests pass single-threaded. The former `cache_eviction_is_bounded_and_short_strings_do_not_evict_sources` asserted `len() <= CACHE_ENTRIES`, which is now false by design, so it is replaced by `indexes_survive_any_number_of_interleaved_strings` — 16 strings, four times the old capacity, asserting every index survives, still answers correctly on a second pass, and is reclaimed by the prune hook. It keeps that test's other, capacity-independent invariant: a one-character string from `char_at` must not disturb its source's index. `gc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check` fails identically with and without this change (same panic site, `copy_slot_decode.rs:135`), verified by running it alone against both trees; it is pre-existing and unrelated. --- changelog.d/10688-per-string-index.md | 6 + .../src/string/char_ops/utf16_index.rs | 108 +++++++++--------- .../src/string/char_ops/utf16_index/tests.rs | 38 +++++- 3 files changed, 96 insertions(+), 56 deletions(-) create mode 100644 changelog.d/10688-per-string-index.md diff --git a/changelog.d/10688-per-string-index.md b/changelog.d/10688-per-string-index.md new file mode 100644 index 0000000000..1c45c5380a --- /dev/null +++ b/changelog.d/10688-per-string-index.md @@ -0,0 +1,6 @@ +The UTF-16 index no longer lives in a fixed four-slot cache that evicted +round-robin. Interleaving indexed access across more strings than it held +rebuilt the index from scratch on every access — 1,224x slower from the fifth +string onward, as a step function. Entries now live until their string dies and +are reclaimed by the collector's existing prune hook, so the cliff cannot occur +at any number of strings (#10688). diff --git a/crates/perry-runtime/src/string/char_ops/utf16_index.rs b/crates/perry-runtime/src/string/char_ops/utf16_index.rs index e6fb4a4ead..171ba0a280 100644 --- a/crates/perry-runtime/src/string/char_ops/utf16_index.rs +++ b/crates/perry-runtime/src/string/char_ops/utf16_index.rs @@ -11,7 +11,6 @@ use super::*; use std::cell::RefCell; const CHECKPOINT_BYTES: usize = 128; -const CACHE_ENTRIES: usize = 4; #[derive(Clone, Copy, Default)] struct Position { @@ -98,24 +97,25 @@ fn decode_step(bytes: &[u8], i: usize) -> (usize, usize, u32) { wtf8_step(bytes, i) } -struct IndexCache { - entries: [Index; CACHE_ENTRIES], - hot: usize, - next: usize, -} +/// #10688: an owner-keyed map rather than a fixed array of slots. +/// +/// The array held `CACHE_ENTRIES` indexes and evicted round-robin, so a +/// program interleaving indexed access across more strings than that evicted +/// the entry it was about to need on every single access and rebuilt from +/// scratch forever — measured at 1,224x once K exceeded the slot count, with +/// no gradual degradation. Capacity is the defect, so there is no capacity: +/// entries live until their string dies, and `prune_dead_utf16_indexes` +/// (already driven by the collector) reclaims them. +/// +/// The map is keyed by a string identity the GC *rewrites* when it relocates +/// an object, so `scan_utf16_index_roots_mut` must rehash after the visitor +/// runs — see there. +type IndexCache = crate::fast_hash::PtrHashMap; -impl Default for IndexCache { - fn default() -> Self { - Self { - entries: std::array::from_fn(|_| Index::default()), - hot: 0, - next: 0, - } - } -} crate::perry_thread_local! { - static UTF16_INDEX_CACHE: RefCell = RefCell::new(IndexCache::default()); + static UTF16_INDEX_CACHE: RefCell = + RefCell::new(crate::fast_hash::new_ptr_hash_map()); } /// Caller has validated the header and UTF-16 index. Small strings bypass the @@ -138,19 +138,16 @@ pub(super) fn boundary_at(s: *const StringHeader, idx: usize) -> Option<(usize, UTF16_INDEX_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); let owner = s as usize; - let slot = if cache.entries[cache.hot].owner == owner { - cache.hot - } else if let Some(slot) = cache.entries.iter().position(|entry| entry.owner == owner) { - slot - } else { - let slot = cache.next; - cache.next = (slot + 1) % CACHE_ENTRIES; - slot - }; - cache.hot = slot; - let entry = &mut cache.entries[slot]; let utf16_len = unsafe { (*s).utf16_len }; - if entry.owner != owner || entry.byte_len != byte_len || entry.utf16_len != utf16_len { + let entry = cache.entry(owner).or_insert_with(|| Index { + owner, + byte_len, + utf16_len, + ..Index::default() + }); + // A uniquely owned string can be appended to in place, which + // invalidates every recorded offset. + if entry.byte_len != byte_len || entry.utf16_len != utf16_len { *entry = Index { owner, byte_len, @@ -181,19 +178,16 @@ pub(super) fn unit_at(s: *const StringHeader, idx: usize) -> Option { UTF16_INDEX_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); let owner = s as usize; - let slot = if cache.entries[cache.hot].owner == owner { - cache.hot - } else if let Some(slot) = cache.entries.iter().position(|entry| entry.owner == owner) { - slot - } else { - let slot = cache.next; - cache.next = (slot + 1) % CACHE_ENTRIES; - slot - }; - cache.hot = slot; - let entry = &mut cache.entries[slot]; let utf16_len = unsafe { (*s).utf16_len }; - if entry.owner != owner || entry.byte_len != byte_len || entry.utf16_len != utf16_len { + let entry = cache.entry(owner).or_insert_with(|| Index { + owner, + byte_len, + utf16_len, + ..Index::default() + }); + // A uniquely owned string can be appended to in place, which + // invalidates every recorded offset. + if entry.byte_len != byte_len || entry.utf16_len != utf16_len { *entry = Index { owner, byte_len, @@ -207,19 +201,27 @@ pub(super) fn unit_at(s: *const StringHeader, idx: usize) -> Option { pub(crate) fn scan_utf16_index_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { UTF16_INDEX_CACHE.with(|cache| { - for entry in &mut cache.borrow_mut().entries { - visitor.visit_metadata_usize_slot(&mut entry.owner); + let mut cache = cache.borrow_mut(); + // The visitor may relocate the string each entry describes, which + // changes the very address the map is keyed by. Drain first, let the + // owners be rewritten, then reinsert so the keys and the `owner` + // fields agree again. + let mut moved: Vec<(usize, Index)> = cache.drain().collect(); + for (key, index) in &mut moved { + visitor.visit_metadata_usize_slot(key); + index.owner = *key; + } + for (key, index) in moved { + cache.insert(key, index); } }); } pub(crate) fn prune_dead_utf16_indexes(is_dead_owner: &dyn Fn(usize) -> bool) { UTF16_INDEX_CACHE.with(|cache| { - for entry in &mut cache.borrow_mut().entries { - if entry.owner != 0 && is_dead_owner(entry.owner) { - *entry = Index::default(); - } - } + cache + .borrow_mut() + .retain(|&owner, _| owner != 0 && !is_dead_owner(owner)); }); } @@ -231,13 +233,15 @@ thread_local! { #[cfg(test)] pub(crate) fn test_utf16_index_entries() -> Vec<(usize, usize)> { UTF16_INDEX_CACHE.with(|cache| { - cache + let mut entries: Vec<(usize, usize)> = cache .borrow() - .entries .iter() - .filter(|entry| entry.owner != 0) - .map(|entry| (entry.owner, entry.checkpoints.len())) - .collect() + .filter(|(&owner, _)| owner != 0) + .map(|(&owner, index)| (owner, index.checkpoints.len())) + .collect(); + // HashMap iteration order is not stable; callers compare snapshots. + entries.sort_unstable(); + entries }) } diff --git a/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs b/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs index 52d8b5ce2e..c82c6b8f46 100644 --- a/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs +++ b/crates/perry-runtime/src/string/char_ops/utf16_index/tests.rs @@ -148,10 +148,22 @@ fn cache_distinguishes_strings_and_invalidates_in_place_appends() { prune_dead_utf16_indexes(&|_| true); } +/// #10688: the index used to live in a fixed four-slot array that evicted +/// round-robin, so interleaving indexed access across more strings than that +/// rebuilt from scratch on every access — 1,224x, as a step function at the +/// fifth string. There is no capacity now, so this asserts the replacement +/// guarantee: **an index survives no matter how many other strings are +/// indexed alongside it.** +/// +/// It also keeps the original invariant this test carried, which is unrelated +/// to capacity and still load-bearing: consuming a one-character string +/// produced by `char_at` must not disturb the source string's index. #[test] -fn cache_eviction_is_bounded_and_short_strings_do_not_evict_sources() { +fn indexes_survive_any_number_of_interleaved_strings() { prune_dead_utf16_indexes(&|_| true); - for i in 0..CACHE_ENTRIES * 3 { + const STRINGS: usize = 16; // comfortably past the old four-slot capacity + let mut sources = Vec::new(); + for i in 0..STRINGS { let text = format!( "{}{}", "中".repeat(256), @@ -162,10 +174,28 @@ fn cache_eviction_is_bounded_and_short_strings_do_not_evict_sources() { let before = test_utf16_index_entries(); let ch = js_string_char_at(s, 256); assert_eq!(js_string_char_code_at(ch, 0), (0x400 + i) as f64); - assert_eq!(test_utf16_index_entries(), before); - assert!(before.len() <= CACHE_ENTRIES); + assert_eq!( + test_utf16_index_entries(), + before, + "a short string from char_at must not disturb the source's index" + ); + sources.push((s, 0x400 + i)); + } + // Every index is still resident: no eviction happened at any depth. + assert_eq!( + test_utf16_index_entries().len(), + STRINGS, + "all {STRINGS} indexes must survive; the old array held only four" + ); + // And every one still answers correctly, cheaply, in a second pass. + for (s, expected) in &sources { + assert_eq!(js_string_char_code_at(*s, 256), *expected as f64); } prune_dead_utf16_indexes(&|_| true); + assert!( + test_utf16_index_entries().is_empty(), + "the collector's prune hook must reclaim them" + ); } /// #10656: `codePointAt` used to walk the WTF-8 payload from byte 0 on every