Skip to content

perf(markdown): pair parenthesized autolinks in one pass - #430

Closed
PathGao wants to merge 1 commit into
masterfrom
perf/parenthesized-autolinks-linear
Closed

perf(markdown): pair parenthesized autolinks in one pass#430
PathGao wants to merge 1 commit into
masterfrom
perf/parenthesized-autolinks-linear

Conversation

@PathGao

@PathGao PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

process_parenthesized_autolinks — the pass that exists because of #290 — ran one scan per candidate. For every ( immediately followed by http://, https:// or ftp:// it walked forward looking for the balancing ), stopping at the first whitespace:

let mut depth = 1usize;
for (offset, ch) in url_tail.char_indices() {
    if ch.is_whitespace() { break; }
    match ch {
        '(' => depth += 1,
        ')' => { depth -= 1; if depth == 0 { closing = Some(url_start + offset); break; } }
        _ => {}
    }
}

That walk has no early exit when the run holds more ( than ). depth only goes up, so it runs to the end of the whitespace-free run and reports "unclosed". The outer loop then advances scan_from by 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://a repeated with no closing paren.

run length before after
100 B 11.7 ms 3.3 ms
300 B 23.1 ms 2.1 ms
1 KB 65.7 ms 1.9 ms
3 KB 185 ms 2.2 ms
10 KB 588 ms 2.0 ms
30 KB 1 816 ms 2.2 ms
100 KB 6 050 ms 2.3 ms
300 KB 24 444 ms 2.8 ms
1 MB (one run) 63 255 ms 2.7 ms

Before: linear in run length at fixed document size, i.e. quadratic. After: flat.

Growth in document size, one run. Doubling the input:

bytes before after
10 000 7.3 ms 0.04 ms
20 000 36.5 ms 0.08 ms
40 000 117 ms 0.13 ms
80 000 532 ms 0.24 ms
160 000 2 178 ms 0.38 ms
320 000 9 131 ms 0.92 ms

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_region is consulted after the scan, so 320 KB of 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:

shape before after
one (https://…)text link per line (the #290 shape) 3.86 ms 6.15 ms
Wikipedia links with a real inner pair, …/Mercury_(planet) 4.63 ms 6.40 ms
(https://a)b with no whitespace at all 7.83 ms 8.87 ms
plain prose, no parens 2.52 ms 0.13 ms
2.5 M bare (, no URL 14.6 ms 0.13 ms

Link-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 and code_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 many url(https://…) — but those are balanced, so each candidate closes in ~15 bytes and the cost stays linear (the (https://a)b row above is exactly that shape).

Yes by opening a file. Markpad opens arbitrary .md from 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 old break enforced, since a bare URL never spans whitespace.

The outer loop is then a for over 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_from still 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 (Unicode White_Space), as before. The byte loop therefore lists U+000B explicitly — u8::is_ascii_whitespace omits it — and decodes any byte ≥ 0x80 so that U+00A0, U+2028 and U+3000 still end a run.

Output is unchanged, and the tests say so

mod tests keeps the old scan verbatim as scan_parenthesized_autolinks and asserts the new pass is byte-identical to it, including which Cow variant comes back (a needless Owned would 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:

  • 49 hand-written adversarial inputs: unbalanced ( 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;
  • every entry of LINE_CONTRACT_CORPUS, in LF and CRLF;
  • exhaustive enumeration: every arrangement of (, ), 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 .md in the repo, scripts/renderProtocolFixtures.ts, the 29 math-corpus cases, lib.rs itself, 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_alone and parenthesized_autolink_keeps_a_balanced_pair_inside_the_url (the Wikipedia case, through convert_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_autolinks stays in line_preserving_transforms(), so every_convert_markdown_preprocessing_step_is_registered, every_preprocessing_step_preserves_source_line_numbers and the_whole_preprocessing_pipeline_preserves_source_line_numbers all 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:

See (https://www.speedtest.net/awards/united_states/)for more information.
  → href="https://www.speedtest.net/awards/united_states/)for"

See (https://en.wikipedia.org/wiki/Mercury_(planet))then.
  → href="https://en.wikipedia.org/wiki/Mercury_(planet))then"

comrak's autolink extension still produces exactly #290. The pass stays.

The regression test is wall-clock, deliberately

parenthesized_autolinks_stay_linear_in_the_longest_run feeds 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:

debug (cargo test) release
one pass 3 ms 0.8 ms
scan per candidate 152 s 9.1 s

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_ranges still 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_runs contains a find over 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.
  • Link-dense documents are ~1.2–1.6x slower (≈2 ms per 2.5 MB), as tabulated above. Recovering that would mean SIMD-scanning for parens and whitespace separately; it was judged not worth the complexity against a pass that is already sub-10 ms on inputs comrak spends hundreds of milliseconds on.
  • Memory is now O(candidates) rather than O(1) — 24 bytes per (<scheme>:// in the document, ~840 KB for the 320 KB adversarial input. Linear, and unmeasurable on real files, but it is new allocation.
  • No behaviour was changed, so nothing that was already wrong about which parenthesized URLs get rewritten is any different. In particular the pass still ignores code_region_ranges until 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

`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>
@PathGao

PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

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.

input before after
2.56 MB of #290-shaped links 3.86 ms 6.15 ms
Wikipedia-style nested parens 4.63 ms 6.40 ms
link-dense documents generally 1.2–1.6× slower

The quadratic case is real and reproduces, but it needs a single whitespace-free run carrying many (http:// openers whose parens never balance. As the PR body already says, that is not reachable by writing Markdown; minified CSS url(https://…) is balanced and stays linear. Trading a measured regression on ordinary documents for protection against a shape nobody produces is the wrong trade for this project.

The costs are not small either: 377 added lines, the previous implementation kept verbatim in mod tests as a differential reference, and a wall-clock assertion — the kind of test most likely to flake on a shared runner.

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 contains("://") early bail, not from the rewrite. It is not worth a PR on its own: 8 ms per keystroke is noise next to comrak and mask_math_spans, which each cost tens of milliseconds on that pipeline.

What is worth following up, and what this work actually surfaced: code_region_ranges is rebuilt from scratch by each of the four preprocessing passes, roughly 1 ms/MB. That is a cost ordinary documents pay on every keystroke, unlike the case this PR addressed. Worth measuring before anyone writes code for it.

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.

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.

1 participant