perf(markdown): pair parenthesized autolinks in one pass - #430
Conversation
`process_parenthesized_autolinks` ran one scan per `(<scheme>://` candidate, walking forward for the balancing `)` and stopping at the first whitespace. That walk has no early exit when the run holds more `(` than `)`, so in a whitespace-free run with no closing paren every candidate walked to the end of the run: cost quadratic in the length of the longest run, and this pass runs on every keystroke via `convert_markdown`. At a fixed 1 MB of input, 100-byte runs cost 12 ms and one 1 MB run cost 63 s. Ordinary prose is unaffected — a 2.5 MB document of normal parenthesized links was already linear at 3.9 ms — so this is reachable by a generated or hostile `.md`, not by writing one. The pairing now comes from a single left-to-right pass. Each candidate is pushed with the nesting depth its `)` has to bring the run back to, so the `)` that pops it is by construction the first one at which the depth relative to that `(` returns to zero — the same `)` the old scan found. Whitespace clears the pending candidates, which is the same boundary the old scan's `break` enforced. Output is unchanged, and `mod tests` now says so mechanically: the old scan is kept verbatim as the reference and asserted byte-identical (`Cow` variant included) over 49 adversarial inputs, the line-contract corpus in both LF and CRLF, and every arrangement of parens, schemes, whitespace and backticks up to five pieces long. The same comparison over 2.2M inputs offline — every `.md` in the repo, the render fixtures, the math corpus, exhaustive enumeration and 2M random documents — found no difference either. #290 stays fixed and the line-number contract of #389 is untouched: every rewrite is still confined to the bytes between a `(` and its `)`, neither of which can be a newline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing this. The measurements do not support it, and they are in the PR body above rather than hidden — so this is a decision the numbers made, not a change of mind. On documents people actually have, this is slower, not faster.
The quadratic case is real and reproduces, but it needs a single whitespace-free run carrying many The costs are not small either: 377 added lines, the previous implementation kept verbatim in The one genuine win on real input — paren-free prose going from 11.7 ms to 3.3 ms at 1 MB — comes from a separable two-line What is worth following up, and what this work actually surfaced: The sibling PR #431 stays open on its own merits — that one is reachable by ordinary use (a maths-heavy document at 2 000 formulas costs 62 ms per keystroke, and drops to 5 ms) with no regression anywhere measured. |
process_parenthesized_autolinks— the pass that exists because of #290 — ran one scan per candidate. For every(immediately followed byhttp://,https://orftp://it walked forward looking for the balancing), stopping at the first whitespace:That walk has no early exit when the run holds more
(than).depthonly goes up, so it runs to the end of the whitespace-free run and reports "unclosed". The outer loop then advancesscan_fromby one byte (scan_from = url_start) and the next(in the same run repeats the whole walk.So the cost is quadratic in the length of the longest whitespace-free run, not in the length of the document. That distinction is the whole story here, and it is why ordinary documents are not affected.
Measured
Release build, M-series laptop, the pass alone (no comrak).
Growth in run length, at a fixed ~1 MB document. Each line is the same 1 MB split into runs of the stated size, every run being
(http://arepeated with no closing paren.Before: linear in run length at fixed document size, i.e. quadratic. After: flat.
Growth in document size, one run. Doubling the input:
Before ~4.1x per doubling (O(n²)); after ~1.9x per doubling (O(n)).
Being inside a fenced code block does not help:
in_code_regionis consulted after the scan, so320 KBof the same shape inside a fence cost 5 510 ms before and 0.49 ms after.Realistic documents, 2.56 MB each. These were linear before and stay linear:
(https://…)textlink per line (the #290 shape)…/Mercury_(planet)(https://a)bwith no whitespace at all(, no URLLink-dense documents get slower by ~1.2–1.6x — about 2 ms per 2.5 MB. That is the cost of the extra byte pass, which is not amortised away when the old scans were already short. It is stated rather than hidden because it is a real trade; at these sizes comrak itself is two orders of magnitude more expensive. Documents with no URL at all get faster because a
contains("://")check now skips both the paren pass andcode_region_ranges.Can a user reach it?
Not by writing Markdown. The trigger needs a single whitespace-free token, tens of kilobytes long, containing many
(http://-style openers whose parens never balance going forward. Prose does not do this. A minified CSS blob pasted into a fence does contain manyurl(https://…)— but those are balanced, so each candidate closes in ~15 bytes and the cost stays linear (the(https://a)brow above is exactly that shape).Yes by opening a file. Markpad opens arbitrary
.mdfrom disk, Finder and file associations, and this pass runs on every render. A file crafted or generated with the shape above hangs the render thread on open and on every subsequent keystroke — 63 s for 1 MB. So: a denial-of-service on opening a hostile or machine-generated document, not a freeze users hit while typing. Framing it as a general "editor freeze" fix would be overselling it.The fix
One left-to-right pass,
parenthesized_url_candidates, produces every candidate(paired with its)up front. Each candidate is pushed on a small stack with the nesting depth its closing)has to bring the run back to; a)pops whatever is waiting at the depth it lands on. That)is by construction the first position where the depth relative to that(returns to zero, which is precisely what the old scan searched for. Whitespace clears the stack — the same boundary the oldbreakenforced, since a bare URL never spans whitespace.The outer loop is then a
forover that list with an O(1) body, so the pass is O(n) time and O(candidates) memory.The rewrite loop keeps every decision the old one made, in the same order:
scan_fromstill jumps past a closing paren (so a(the previous rewrite swallowed is skipped), an unclosed candidate is still just passed over, and the adjacency and code-region checks are unchanged. Only the search for the)changed.The whitespace test is
char::is_whitespace(UnicodeWhite_Space), as before. The byte loop therefore listsU+000Bexplicitly —u8::is_ascii_whitespaceomits it — and decodes any byte ≥0x80so that U+00A0, U+2028 and U+3000 still end a run.Output is unchanged, and the tests say so
mod testskeeps the old scan verbatim asscan_parenthesized_autolinksand asserts the new pass is byte-identical to it, including whichCowvariant comes back (a needlessOwnedwould be a silent allocation regression). The reference is the executable definition of what this pass means; any future rewrite has to reproduce it too.It runs over:
(and), deeply nested parens, a paren run with no URL,…/Mercury_(planet)and its unbalanced twin, a candidate nested inside another, inside a code span, inside a fenced block, inside an unclosed fence, non-ASCII text and non-ASCII whitespace beside the parens,HTTP://(correctly not matched), the pattern at the very start and very end of the document, and CRLF;LINE_CONTRACT_CORPUS, in LF and CRLF;(,),http://,a, space, newline and backtick up to five pieces long, and every arrangement of(http://a,),b, space,(and backtick up to five pieces long — ~29 000 documents.Offline, the same comparison was run over 2 196 278 inputs with no difference: every
.mdin the repo,scripts/renderProtocolFixtures.ts, the 29 math-corpus cases,lib.rsitself, each also in CRLF, plus the exhaustive enumerations at greater depth and 2 000 000 random documents drawn from a paren/scheme/whitespace/code-fence/CJK alphabet.Two behaviour tests were added on top of the differential:
parenthesized_autolinks_leave_code_regions_aloneandparenthesized_autolink_keeps_a_balanced_pair_inside_the_url(the Wikipedia case, throughconvert_markdown).autolink_inside_parentheses_stops_before_adjacent_text— the #290 regression test — is untouched and still green, still asserting the same three things.The line-number contract (#389)
Unchanged and unchanged by construction: every rewrite is confined to the bytes between a
(and its), and neither can be a newline (a newline is whitespace and ends the run).process_parenthesized_autolinksstays inline_preserving_transforms(), soevery_convert_markdown_preprocessing_step_is_registered,every_preprocessing_step_preserves_source_line_numbersandthe_whole_preprocessing_pipeline_preserves_source_line_numbersall still cover it. No line count changes.comrak 0.54 does not do this job
Checked against the parser as it stands after #426, with the pass disabled:
comrak's
autolinkextension still produces exactly #290. The pass stays.The regression test is wall-clock, deliberately
parenthesized_autolinks_stay_linear_in_the_longest_runfeeds 320 KB of the pathological shape and fails over 4 s.A structural assertion was preferred and could not be made to work: nothing about the output distinguishes a linear implementation from a quadratic one — only the work does, and a future rewrite that reintroduces a nested scan would still produce identical bytes and still satisfy the differential test above. So the budget is sized from measurement instead, in the profile CI actually runs:
cargo test)Budget 4 s. Failing wrongly needs a machine ~1 300x slower than the one measured; passing wrongly needs one ~38x faster. Neither verdict is close, which is the property that keeps it out of the flaky-test category.
Not covered
code_region_rangesstill runs over the whole document and is still the dominant per-keystroke cost for ordinary files (~1 ms/MB). Four passes each call it and each rebuilds it. Sharing one computation across the pipeline is a separate change.pair_inline_code_runscontains afindover subsequent backtick runs that looks quadratic. It is not reachable quadratically — making m runs never pair requires Ω(m²) bytes of input — but it was examined, not fixed.(<scheme>://in the document, ~840 KB for the 320 KB adversarial input. Linear, and unmeasurable on real files, but it is new allocation.code_region_rangesuntil after it has found a closing paren; the check order was preserved rather than improved, so the byte-equality claim would hold.🤖 Generated with Claude Code