Repository navigation
STRING-INDEX-0: carry ASCII-ness on RcStr so char_at is O(1) - #8360
Merged
Merged
Conversation
Value::Str now wraps RcStr { rc: Rc<str>, is_ascii: bool }, computing
is_ascii once at construction (DESIGN §5 construction-over-validation)
instead of rescanning the whole string on every char_at/substring/
string_length call. That per-call s.is_ascii() rescan was the O(n)
cost hiding inside what looked like an O(1) accessor, turning repeated
indexing over a large string into O(n^2) (STRING-INDEX-0). This
supersedes the pointer-keyed ASCII cache withdrawn in CHARAT-0 for
aliasing on allocation reuse.
v1_rt.rs's char_at/substring/string_length gain _ascii_aware variants
taking the precomputed flag; all interpreter call sites (v1_interpreter.rs)
route through them via RcStr::is_ascii(). RcStr::ptr_eq lets consumers
that need pointer-identity semantics (e.g. the value_str_rc_semantic_parity
sharing control from #8306) keep them.
Evidence:
- char_at_unicode_witness_test.dag (green): ASCII and non-ASCII behavior
parity, unchanged semantics.
- value_str_rc_semantic_parity_tests::*ptr_eq* (green, from #8306): Rc
sharing semantics preserved across the RcStr wrap.
- New isolated char_at_scaling_probe (not floor-enrolled, scaffold with
a declared dissolution trigger): a single char_at call through the
interpreter, repeated at varying position and varying string length,
with no JSON parsing or list accumulation in the loop. Remote receipt:
mean per-call cost flat at ~3.3-3.8us across string lengths 10K-2M
chars (200x range) and positions spanning 0-99% of each length, with
no growth trend against either axis. This is the char_at-specific O(1)
evidence; end-to-end JSON-parse scaling is a different measurement.
Scope note on the JSON-parse end-to-end scaling probe
(json_parse_scaling_probe.rs): its exponent is UNCHANGED by this PR,
and that is expected, not a regression. The JSON parser accumulates
list members via list_push into what is empirically a native
Value::List on every call (a new `counters` mode added to the probe
this PR, reporting MutationCounters, shows list_push_items_copied = 0
at 20KB/40KB/80KB while list_push_calls scales linearly with member
count -- refuting the initially-hypothesized value_to_list_carrier /
free_monoid_to_vec O(m) fallback-copy explanation for that probe's
prior ~2.06 measured exponent). The true cause of the JSON end-to-end
exponent is therefore still open, sits in the List carrier / parser
accumulation path (not String), and is out of this PR's scope --
list_push, value_to_list_carrier, and free_monoid_to_vec are untouched
here and are being raised as a separate work item. The cold 507KB
parse and Codex-materialization measurements are likewise deferred to
a future joint receipt once that List-side defect is diagnosed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UKuY9M5MtTB4e5fJKUysUs
…ntime_rust.dag The ascii_aware split (STRING-INDEX-0) was only applied to the checked-in v1_rt.rs, leaving runtime_rust.dag and its stage0 transliteration (v1_compiler_runtime_rust.rs) on the old plain form. That dual representation is what the regen CI gate caught: a fresh self-compile regenerates v1_rt.rs from the old form and reverts it. Regenerated stage0 to a fixed point (build -> write pass 1 -> rebuild against the newly-baked runtime_rust.dag -> write pass 2) so all three files agree: runtime_rust.dag is now the single source of the char_at_ascii_aware / string_length_ascii_aware / substring_ascii_aware split, and both generated files are its direct, converged output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UKuY9M5MtTB4e5fJKUysUs
The Value::Str arm still did an unconditional s.chars().count() scan after string_length_ascii_aware() landed for free_call.string_length. Use the same O(1)-for-ASCII helper here too (review on gunbc#8360). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UKuY9M5MtTB4e5fJKUysUs
DESIGN §4c: dissolution conditions belong in a typed carrier, not a bare `String` note. §3: cite the actual symbol, not another file's marker by imprecise prose. §5: one dissolution obligation covers one independently removable unit — this probe measures a different primitive (char_at cost in isolation) than json_parse_scaling_probe.rs (end-to-end parse), on a potentially different lifetime, so it gets its own DissolutionCondition rather than sharing json_parse's trigger. The probe is retained evidence per §4b, not scaffold debt — framed as std.disposition.Terminal — with an independent std.dissolution UnboundDissolution citing its own real grep receipt, CHAR_AT_SCALING_PROBE_SCAFFOLD_MARKER in char_at_scaling_probe.rs (distinct from json_parse's JSON_PARSE_SCALING_PROBE_SCAFFOLD_MARKER). Mirrored the same correction into the bin's own doc comment. Verified by remote build: the updated module resolves and the probe still runs, reproducing char_at's flat per-call cost across position and string length. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UKuY9M5MtTB4e5fJKUysUs
…ause eager-koi-458 review: "or when this measurement refutes the hypothesis" named no observable fact, unlike the good first clause. Point it at the probe's own printed TSV and the specific trend (mean_call_us growing with string_len) that would constitute a refutation, mirrored in both the .dag description and the .rs doc comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UKuY9M5MtTB4e5fJKUysUs
briansrls
pushed a commit
that referenced
this pull request
Aug 18, 2026
#8390) * Fix Str `.length()` falling through to free_monoid_to_vec O(n²) materialization. Method-call `.length()` on native `Value::Str` now uses `string_length_ascii_aware`, matching the existing free-call `length`/`string_length` arms. Without this, JSON parsing's O(n) `.length()` probes on the input buffer materialized one Value per codepoint on each call, pinning ~8 GiB RSS on ~500 KB inputs and blocking materialize_codex_runtime_bundle wet receipts. Co-authored-by: Cursor <cursoragent@cursor.com> * Declare §4b ceiling on native_len Str arm for non-ASCII O(n) length. Co-authored-by: Cursor <cursoragent@cursor.com> * Use &str coercion for native_len on Rc<str> until RcStr re-lands on main. main lost the RcStr carrier (#8357 squash over #8360); s.as_str() on Rc<str> resolves to unstable str::as_str. Coerce via &s — same semantics, no RcStr dependency in this PR. Co-authored-by: Cursor <cursoragent@cursor.com> * Correct native_len doc: free-call arms use chars().count(), not ascii_aware. Review 53039 noted the prior comment overstated parity with free_call.length/ string_length; those paths already avoided free_monoid_to_vec without this arm. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
added a commit
that referenced
this pull request
Aug 25, 2026
…uted flag names an RcStr carrier that does not exist (#9212) * char_at's ascii-aware split has no producer on main: delete it and bound the ASCII test by pos `v1_rt::char_at_ascii_aware` / `string_length_ascii_aware` / `substring_ascii_aware` take a precomputed `is_ascii` flag, and their doc comment names the producer of that flag as "the `RcStr` carrier fact". No `RcStr` exists in this tree: `Value::Str` is `Rc<str>`, and every caller -- `char_at`, `string_length`, `substring`, and the one interpreter call site -- supplies `s.is_ascii()`, an O(n) whole-string rescan computed fresh per call. The split is therefore unroutable: it is three public functions whose parameter has no producer, plus a comment asserting a carrier that does not exist. Provenance, because the obvious reading (someone deleted RcStr) is wrong: b1775d8 (#8360) landed BOTH halves and is NOT an ancestor of main. main's history is rooted at 67437fc (#8833), a wholesale seed re-import whose tree already carries the runtime split (`src/v1/runtime_rust.dag` +826) beside an interpreter with no `RcStr` (+16717, zero occurrences). The flag has never had a producer anywhere in main's reachable history, and #8360's O(1) claim has never been true of this tree. The repair, at the single authority `src/v1/runtime_rust.dag` (mirrors `v1_rt.rs` / `v1_compiler_runtime_rust.rs` follow by regen): - Delete the `_ascii_aware` triplet. Nothing supplies a flag other than `s.is_ascii()`, so the parameter is a second representation of a fact the function can read itself (DESIGN §2/§3), and the comment on it is the §4b inflation case -- a carrier named for a rung the tree does not occupy. - Bound the ASCII test by the requested position instead of by the whole string. A leading run of ASCII bytes makes the byte offset equal the code-point offset, so `char_at` examines `bytes[..=pos]` and `substring` examines `bytes[..end]`, never the tail. Cost drops from O(n) + O(pos) to O(min(pos, n)) -- the fallback's own cost. Semantics are unchanged: where the old form fell back to `chars()` because the STRING contained a multibyte char, the new form takes the byte path only when the PREFIX up to the requested index is ASCII, which is exactly the condition under which byte index equals code-point index. - Route the interpreter's `native_len` `Value::Str` arm through `v1_rt::string_length` and fix its doc comment, which named the deleted helper. - Delete `gunbc.char_at_scaling_probe_support`. Its `DissolutionCondition` names `src/v1/stage0/src/bin/char_at_scaling_probe.rs`, deleted by #9160, and its trigger is "when char_at's O(1) property is floor-enrolled" -- a property this tree does not have. It is unconsumed (census row in docs/plans/unconsumed-module-residue-disposition.md) and its subject is gone. RESIDUAL, DECLARED RATHER THAN CLAIMED CLOSED: a left-to-right walk of a string is still O(n^2), now with a smaller constant rather than a different shape. A single call cannot be O(1) without the whole-string ASCII fact, which needs a carrier on the string value; that carrier -- or a cursor surface that does not re-index from zero -- is this class's next-rung trigger, and it is recorded in the `char_at` doc comment rather than left to be rediscovered. This change lowers no rung: the deleted split was inert, so nothing it guaranteed is lost. Evidence: dag/test/claim/char_at_unicode_witness_test.dag (SubstrateInputsOnly, floor-routed) gains the discriminating control for the prefix-bounded path -- "ab" + U+00E9 + "c" is 5 bytes and 4 code points, so a byte-offset implementation returns U+00A9 at index 3 and "c" at index 4 where code-point indexing returns "c" at 3 and "" at 4, with the same split applied to `substring`. * Regen: converge v1_compiler_runtime_rust.rs with the edited runtime_rust.dag authority (pass 1) Pass 1 of the stage0 two-pass convergence. `--required-regen` regenerated `v1_compiler_runtime_rust.rs` -- the stage0 transliteration of `src/v1/runtime_rust.dag` -- from the edited authority; this installs that candidate byte-for-byte. `v1_rt.rs` is emitted BY this mirror, so its pass-1 candidate was still the old `_ascii_aware` text and converges only on pass 2, after a rebuild against the mirror installed here. * Drop the substring assertion: its RED was never observed, so it is not coverage The witness gained three assertions for the prefix-bounded fast path. Two of them are demonstrated discriminating: with `char_at`'s prefix check removed and the binary rebuilt from scratch (perturbation confirmed present in source), `char_at_agrees_across_the_ascii_prefix_boundary`, `char_at_past_the_multibyte_char_is_not_a_byte_offset` and the pre-existing `char_at_indexes_code_points_on_multibyte_text` all go RED. The third, `substring_agrees_across_the_ascii_prefix_boundary`, does not. Its own perturbation -- `if bytes[..out_end].is_ascii()` replaced by `if true`, so the byte-slice path is taken unconditionally -- left it GREEN, twice, the second time with the binary deleted first, the build failure-checked rather than piped through `tail`, and the edit grep-proven in source. A follow-up probe that would have printed the returned values panicked in `cli_run.rs` on BOTH arms, so it measured nothing: identical output across arms is the signature of an instrument that did not run, not of agreement. So the mechanism is unexplained. What is NOT in doubt is the assertion's status: a check whose RED has never been observed is not evidence, and shipping it would put it in the worst class DESIGN §4b names -- permanently green as far as anyone can show, and cited as coverage precisely because it is named after the thing it does not test. It is removed rather than kept with a caveat, because a caveat in a PR body does not travel with the test. The `substring` code change stands: it is the same prefix-bounding as `char_at`, semantics-preserving by the same argument (the byte path is taken only when the prefix up to the requested index is ASCII, which is exactly when byte index equals code-point index), and substring is exercised heavily by the existing corpus. What it does not have is a discriminating control of its own, and the witness note now says so in the module rather than leaving a reader to infer coverage from the file's name. OPEN QUESTION, recorded rather than routed around: `substring(s, 0, 3)` on "ab" + U+00E9 + "c" under the unconditional byte path slices `s[0..3]`, which lands inside the two-byte U+00E9 and should panic on a non-char-boundary. It did not. Either that path is not reached by a named-argument `.dag` call, or the panic is absorbed somewhere between the interpreter arm and the claim runner's exit status. The second would be the more serious finding -- a witness that cannot go red because failures are swallowed would affect every witness, not this one -- and it is worth its own lane. --------- Co-authored-by: Brian Searls <briansearls1@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Value::Strnow wrapsRcStr { rc: Rc<str>, is_ascii: bool }, computingis_asciionce at construction (DESIGN §5 construction-over-validation) instead of rescanning the whole string on everychar_at/substring/string_lengthcall. That per-calls.is_ascii()rescan was the O(n) cost hiding inside what looked like an O(1) accessor, turning repeated indexing over a large string into O(n^2) (STRING-INDEX-0). This supersedes the pointer-keyed ASCII cache withdrawn in CHARAT-0 (#8320) for aliasing on allocation reuse.v1_rt.rs'schar_at/substring/string_lengthgain_ascii_awarevariants taking the precomputed flag; all interpreter call sites inv1_interpreter.rsroute through them viaRcStr::is_ascii().RcStr::ptr_eqpreserves pointer-identity semantics for consumers that need it (e.g. thevalue_str_rc_semantic_paritysharing control from #8306).Two small consumer adaptations follow from
Value::Strnow wrappingRcStrinstead of a bareRc<str>:derived_realization_schedule.rsusess.rc()to get an ownedRc<str>, andrecorded_fixture.rsusess.as_ref()for JSON serialization.Evidence
char_at_unicode_witness_test.dag(green): ASCII and non-ASCII behavior parity, unchanged semantics.value_str_rc_semantic_parity_tests::*ptr_eq*(green, from STR-RC-0: share Value::Str storage to eliminate recursive deep-copy memory blowup #8306):Rcsharing semantics preserved across theRcStrwrap.char_at_scaling_probe(not floor-enrolled, scaffold with a declared dissolution trigger, same class as the existingjson_parse_scaling_probe): a singlechar_atcall through the interpreter, repeated at varying position and varying string length, with no JSON parsing or list accumulation in the loop — isolating exactly the one thing this PR changed. Remote receipt (exit 0): mean per-call cost flat at ~3.3–3.8µs across string lengths 10K–2M chars (200x range) and positions spanning 0–99% of each length, with no growth trend against either axis. This is the char_at-specific O(1) evidence.Scope note: JSON end-to-end scaling is unchanged, and that's expected
The existing
json_parse_scaling_probe.rs's end-to-end parse scaling exponent (~2.06, measured pre-PR) is unchanged by this PR. That's not a regression — the JSON parser's dominant cost is a different, pre-existing defect in the List carrier's accumulation path, out of this PR's scope.This PR adds a
countersmode to that probe, reportingMutationCounters(list_push_callsvslist_push_items_copied) around a parse. Result at 20KB/40KB/80KB (2048/4096/8192 members):list_push_items_copied = 0at every size whilelist_push_callsscales linearly with member count. This refutes the initial hypothesis that the exponent was caused byvalue_to_list_carrier'sfree_monoid_to_vecO(m) fallback-copy branch on the push path (that branch is never taken here — the accumulator arrives as a nativeValue::Liston every push, hitting the cheap 0-copy path).Update: the root cause is no longer open — it is diagnosed, with a successor named.
native_len(v1_interpreter.rs), which backsmethod_call.length/method_call.is_empty(x.length(),x.is_empty()), has noValue::Strarm — verified directly in this PR's branch — so a.length()/.is_empty()call on a string falls through tofree_monoid_to_vec, which materializes the entire string into aVec<Value>of chars just to report its count: an O(n) full-string flatten hidden behind what reads as an O(1) accessor, the same failure shape STRING-INDEX-0 fixed forchar_at/substring/string_length, on a sibling call site. This was independently identified bybright-otter-592and is out of this PR's scope: it lives innative_len's dispatch, adjacent to but not inside the List-carrier machinery (list_push,value_to_list_carrier,free_monoid_to_vec) this PR is scoped away from touching.list_push,value_to_list_carrier, andfree_monoid_to_vecremain untouched in this PR; the fix belongs to a successor work item (adding aValue::Strarm tonative_len).The cold 507KB parse and the two Codex-materialization measurements from this work item's original brief are deferred to a future joint receipt once that List-side defect is diagnosed, per explicit scope guidance from the parent session.
Test plan
char_at_unicode_witness_test.dag— green (Unicode/ASCII char_at parity witness).value_str_rc_semantic_parity_tests(cargo test,#8306) — green (Rc sharing/ptr_eq controls).char_at_scaling_probe(new, not floor-enrolled scaffold) — remote run, exit 0, flat ~3.3–3.8µs/call across a 200x length range and full position range: the O(1) receipt for this change.json_parse_scaling_probe --mode counters(new mode on existing scaffold) — remote run, exit 0,list_push_items_copied = 0at all three tested sizes: refutes the free-monoid hypothesis for the (unchanged, out-of-scope) JSON end-to-end exponent.Closes dashboard://work-items/adhoc-c6d0ad7b-28b