Fix quadratic escaped string parsing in escaped_byte_buf - #610
Merged
Conversation
escaped_byte_buf called find('"') once per escape. That call scans from the
cursor to the closing quote, while the cursor only advances past one escape per
iteration, so a string with N escapes rescanned the tail N times, making string
parsing O(N^2) in the number of escapes.
Unlike ron-rs#607 this is not limited to ron::Value / deserialize_any: it affects any
deserialization of a string containing escapes, including the typed path.
The quadratic scan was introduced in ron-rs#534, which replaced a single scan for the
first of '"' / '\\' with "find the closing quote, then look for a backslash
before it". That is genuinely faster for the common no-escape case, so keep it
as the fast path outside the loop; inside the loop only the first of '"' / '\\'
is needed, and scanning just to the nearest delimiter is enough.
Restores linear scaling: a 400KB string of escapes goes from 1652.96ms to
1.54ms (1073x), and the ratio per doubling goes from 3.99 back to 2.02.
Add a regression test asserting on scaling rather than absolute time, so it
stays meaningful on slow or noisy machines: the ratio is ~2 when linear and ~4
when quadratic, and the test fails on the pre-fix parser.
Note on error reporting for invalid input: for an unterminated string whose
invalid escape follows an escaped quote (e.g. "a\"b\c), this reports
InvalidEscape rather than ExpectedStringEnd, which is what the parser did
before ron-rs#534. The outer find('"') is left untouched, so unterminated strings
without escapes still report ExpectedStringEnd as they do today.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
std::hint::black_box is only stable since Rust 1.66 (E0658 on the 1.64 MSRV job); drop it — the .unwrap()ed cross-crate from_str::<Value> can't be elided anyway. Also split the find(['"', '\\']) call across lines to satisfy rustfmt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Fixed the CI: dropped the |
enomado
added a commit
to enomado/ron
that referenced
this pull request
Jul 16, 2026
ron has ~450 correctness tests but none that guard the asymptotic cost of parsing. That gap let ron-rs#534 regress escaped_byte_buf from O(n) to O(n^2) across v0.9.0..=v0.12.2 — four releases — with every test green, and a second O(n^2) entered next_bytes_is_float in v0.12.2 (ron-rs#602). tests/complexity_scaling.rs asserts that parse time grows at most linearly: it doubles the input and checks the time ratio (~2 linear vs ~4 quadratic; threshold 3.0), using min-of-N + median to stay robust to timing noise. The tests are #[ignore]d and run in a dedicated release CI job, so they never slow down or flake the normal test job. Two forms guard the regressions fixed in ron-rs#608 (next_bytes_is_float) and ron-rs#610 (escaped_byte_buf); two linear controls keep the harness honest. On the pre-fix tree (d0e99bc) those two forms report ratio ~4.0 and fail the 3.0 threshold; on current master they are ~2.0 and pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.
Fixes #609.
escaped_byte_bufcallsfind('"')once per escape. That call scans from the cursor tothe closing quote, while the cursor only advances past one escape per iteration —
so a string with N escapes rescans the tail N times, making string parsing O(N²) in the
number of escapes.
Unlike #607, this is not limited to
ron::Value/deserialize_any: it affects anydeserialization of a string containing escapes, including the typed path.
Root cause
The quadratic scan came in with #534, which replaced a single scan for the first of
"/\\:with "find the closing quote, then look for a backslash before it". That is genuinely
faster for the common no-escape case, so this PR keeps it as the fast path outside
the loop. Inside the loop, though, only the first of
"/\\is ever needed — scanningjust to the nearest delimiter is both sufficient and linear.
Affected releases (
git tag --contains 08c691d0): v0.9.0 … v0.12.2.Result
["\n\n\n…"], release build, atd0e99bc:1073x at 400 KB, and the per-doubling ratio goes from ~4 (quadratic) back to ~2
(linear). Throughput: 0.2 MB/s → 260 MB/s.
Both branches already did the same
extend_from_slice, so the fix also makes the loopslightly shorter.
Tests
The regression test asserts on scaling, not absolute time, so it stays meaningful on
slow or noisy CI: the ratio is ~2 when linear and ~4 when quadratic, and the threshold
(3.0) sits far from both. It fails on the pre-fix parser (measured ratio 3.96) and passes
after. Same shape as the test in #608.
The existing suite (453 tests) stays green, plus the 2 new ones.
Note on error reporting for invalid input
For an unterminated string whose invalid escape follows an escaped quote — e.g.
"a\"b\c— this now reportsInvalidEscapeinstead ofExpectedStringEnd. That is whatthe parser did before #534 (verified by running the pre-#534 tree), and it is arguably the
more accurate diagnostic, since there genuinely is an invalid escape.
The outer
find('"')is deliberately left untouched, so the common case — an unterminatedstring with no escapes, e.g. the
(4, "Hello)case pinned inde::tests::forgot_apostrophes— still reports
ExpectedStringEndexactly as today. No existing test changes behaviour.Happy to preserve the current error exactly if you'd prefer; it costs a little extra state
to track the last found quote across iterations.