Skip to content

Fix quadratic float parsing in next_bytes_is_float - #608

Merged
juntyr merged 3 commits into
ron-rs:masterfrom
enomado:fix/607-quadratic-float-parsing
Jul 16, 2026
Merged

Fix quadratic float parsing in next_bytes_is_float#608
juntyr merged 3 commits into
ron-rs:masterfrom
enomado:fix/607-quadratic-float-parsing

Conversation

@enomado

@enomado enomado commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #607.

Problem

Parser::next_bytes_is_float searches the entire remaining input for "..", but clamps the result to the current float-char run:

let raw_float_len = self.next_chars_while_from_len(skip, is_float_char);
let valid_float_len = self.src()[skip..]
    .find("..")                              // scans to EOF
    .map(|i| i.min(raw_float_len))
    .map_or(raw_float_len, |i| i.min(raw_float_len));

Since the result is .min(raw_float_len) anyway, every byte scanned past raw_float_len is wasted. In a document with no ".." in it — the common case — find scans to EOF for every number, making self-describing deserialization (ron::Value / deserialize_any) O(numbers × input_size).

This regressed in v0.12.2 with #602. Same benchmark, only the version differs (--release):

floats v0.12.1 v0.12.2 slowdown
16 000 3.2 ms 335 ms 105×
32 000 6.4 ms 1 397 ms 218×
64 000 12.2 ms (×1.90) 5 645 ms (×4.04) 462×

Fix

Bound the search to the float-char run — the form any_number already uses a few lines above:

let valid_float_len = self.src()[skip..][..raw_float_len]
    .find("..")
    .map_or(raw_float_len, |i| i.min(raw_float_len));

Behaviour-preserving:

  • A match at index >= raw_float_len was already clamped to raw_float_len, so not finding it changes nothing.
  • A ".." cannot straddle the end of the run: that would require src[raw_float_len] == '.', but '.' is a float char (is_float_char) and would have been part of the run.

The now-redundant .map(|i| i.min(raw_float_len)) before .map_or(...) is dropped (map_or already re-applies the same min).

Result: linear again, 64k floats 5 645 ms → 27 ms (207×).

Tests

  • All 453 existing tests pass, including the seven range tests from Add support for number ranges  #602.
  • Adds tests/607_quadratic_float_parsing.rs, which asserts on scaling rather than absolute time so it stays meaningful on slow or noisy CI: the ratio is ~2 when linear, ~4 when quadratic, and the threshold (3.0) sits far from both. It bails out if the baseline is too small to measure.

I verified the test genuinely catches the bug: on the unfixed parser it fails with ratio 3.90, and it passes on the fixed one.

next_bytes_is_float searched the entire remaining input for ".." while
clamping the result to the current float-char run. Since a match at or
beyond raw_float_len is clamped away anyway, every byte scanned past the
run was wasted work: on documents containing no ".." at all, each number
scanned to EOF, making self-describing deserialization O(numbers * input).

This regressed in v0.12.2 with the number range support (ron-rs#602). On a flat
list of floats, v0.12.1 scales linearly while v0.12.2 scales quadratically
(462x slower at 64k numbers, and growing).

Bound the search to the float-char run, matching the form any_number
already uses. This is behaviour-preserving: a ".." cannot straddle the end
of the run, since that would require src[raw_float_len] == '.', and '.' is
a float char that would have been part of the run.

Restores linear scaling: 64k floats go from 5645ms to 27ms (207x).

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.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@juntyr

juntyr commented Jul 15, 2026

Copy link
Copy Markdown
Member

Thanks for the catch and the fix!

@juntyr

juntyr commented Jul 15, 2026

Copy link
Copy Markdown
Member

@enomado It seems like we might need to disable the test on the 1.64

@juntyr

juntyr commented Jul 15, 2026

Copy link
Copy Markdown
Member

Or maybe it could work without the black box hint?

enomado and others added 2 commits July 16, 2026 12:24
black_box is only stable since Rust 1.66, so it broke the 1.64 MSRV build (E0658). Binding the parsed value to _value and letting it drop is enough: from_str::<Value> is a cross-crate, non-inlined call that allocates and is .unwrap()ed, so the parse can't be optimized away.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@juntyr
juntyr merged commit 7213485 into ron-rs:master Jul 16, 2026
10 checks passed
@enomado

enomado commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Went with your second suggestion — dropped the black_box hint instead of gating the test off 1.64, so it keeps running everywhere. That was the only real breakage: std::hint::black_box is only stable since 1.66, so the 1.64 MSRV job hit E0658 (the stable/nightly jobs were just cancelled by fail-fast). The .unwrap()ed cross-crate from_str::<Value> allocates, so the parse still isn't optimized away without the hint. CI is green now.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quadratic float parsing in v0.12.2: next_bytes_is_float scans to EOF per number

2 participants