Repository navigation
fix: quadratic inline parse time on failed link candidates (IJPL-96392) - #233
Conversation
![jetbrains-air[bot]](https://avatars.githubusercontent.com/in/3314082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: request changes — the two blocking inline comments need to be addressed before merge (posted as a comment review because GitHub does not allow this account to formally request changes on its own PR).
Produced by Air Automations. Name: Markdown library: review the PR / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/2269eaa4-da15-4cac-957f-3341151c79ec?run=93e2f7b9-7d8c-4f66-a936-89c5392f1cca
Inline parsing was quadratic on text full of link or image candidates that never form a link. For every `[` or `![`, the link parsers scanned forward for the end of the link text, destination, and title, then threw the scans away when the candidate failed, so the next candidate rescanned the same tokens. With 8,000 repetitions, each of these took seconds per parse, up to ~14 s: - nested brackets such as `[[[a]]]`, `[[[a](](` or `[[[a][]][]` rescanned the link text up to its matching `]` for every `[`; - `[a](b (` or `[a](<` scanned for the end of the title or of the `<...>` destination up to the end of the paragraph, even when a single `)` followed; - `[a](((` without white space scanned the destination up to the end of the word for every candidate. A successful link consumes the tokens it scanned, so only failed candidates need to be cheap. LinkParserUtil.ScanIndex is built once per parsed range list and records where each link part starting at a token ends: the `]` matching a link text, the end of a destination without braces, and the next `>`, `)`, or quote closing a `<...>` destination or a title. The image, inline link, reference link, and GFM math parsers pass it to parseLinkDestination and parseLinkTitle, which look the end up instead of scanning, and collect the link text only once a link is known to be complete. The index is built only when a range list has a link candidate, and its destination and title parts only on first use, so plain text does no extra work. Parse results are unchanged: the trees of 30k random bracket-heavy inputs are identical to master's in both flavours. The shapes above now parse in 6-21 ms and scale linearly, and realistic documents (gitBook, commonMarkSpec, fogChangelog) parse as fast as before. The new performance tests cover these shapes and their image variants. Not covered: the GFM lexer is still quadratic on long runs of autolink starts without white space, such as `http://http://...` or `www.a!www.a!...`, because the optional `userinfo@` prefix of GFM_AUTOLINK keeps its scan going to the end of the run. That needs a separate lexer change. Fixes IJPL-96392. Produced by Air Automations. Name: Markdown library: fix the bug / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/cb6a17f6-e7a9-41a8-9611-7c95c4eb0141?run=e9cbf485-8272-43f8-9187-aab50412fdd2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
6d0bf37 to
36d0b02
Compare
|
I've addressed problems above ^, so should be better now |
|
Codex (@codex) review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36d0b02ee2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
2 participants
Problem
IJPL-96392 reports ~15 seconds of full CPU load after every keystroke in a big (~1k lines) Markdown file, with laggy highlighting. The IDE re-parses the file on each keystroke, so any super-linear parse cost in this library multiplies into exactly that symptom.
Profiling the parser against a battery of inputs found that inline link parsing is quadratic on text with many bracket link candidates that never form a link. Every
[triggers forward token scans (parseLinkText,parseLinkDestination,parseLinkTitle) that are thrown away when the candidate fails, so the next candidate re-scans the same tokens:[[[…]]]:ReferenceLinkParser(and the GFMMathParserlink-range scan) runsparseLinkTextup to the matching]for every opening bracket — measured 1.67 s for 16 KB of input (4× time per size doubling), i.e. the reported ~15 s at realistic file sizes.[...](without a closing):InlineLinkParserscans for a link title up to the end of the paragraph for every candidate — measured 2.67 s for 40 KB.This is the same family as the earlier IJPL-93535 "Freeze in LinkParserUtil" fix (
buildBracketStarts), which pruned unmatched-bracket candidates but not these two shapes.Fix
Successful parses may keep their scans — they consume the tokens they scanned, so their cost amortizes to linear. Only failed candidates need to be cheap:
LinkParserUtil.buildBracketMatchesprecomputes the[→ matching]index map in one pass.ReferenceLinkParserandMathParseruse it to skip the full-reference-link attempt in O(1) unless the raw token right after the matching]is[— a necessary condition the old code only discovered after scanning the whole link text (it.start == linkTextEnd+LBRACKETinsideparseLinkLabel).LinkParserUtil.collectRParenIndicesprecomputes the sorted)token indices in the parsed ranges.InlineLinkParserandMathParserreject a candidate in O(1) when no)exists after its](— a necessary condition forparseInlineLink's finalRPARENcheck.Both pre-checks are pure necessary conditions of the existing parsing code, so parse results are unchanged.
Verification
testNestedMatchedBracketsAreLinear,testLinkCandidatesWithoutClosingParenAreLinear, both CommonMark and GFM flavours) fail on master (808 ms+ per parse vs the 250 ms budget) and pass with the fix at ~6 ms / ~14 ms per parse — a ~200–250× improvement on those inputs.jvmTest+performanceTestsuites: 1582 tests, 0 failures. JS target compiles.Remaining uncertainty / follow-up
The reporter's file is not attached to the issue, so the exact trigger cannot be confirmed; these are the only inline-parser behaviors found that reach the reported magnitude on bracket-heavy text. Two smaller, separate pathologies were also identified and are left out of scope:
http://http://…): after each autolink candidate the DFA stays alive through the rest of a whitespace-free run via the optional{URL_USER_INFO_CHAR}+ "@"userinfo branch (whose char class includes/and:), then falls back./is not valid in an RFC 3986 userinfo, so removing it fromURL_USER_INFO_CHARingfm.flexwould bound the scan, but that requires regenerating_GFMLexer.kt.parseLinkDestinationcan still scan far on whitespace-free runs of unbalanced(— reachable only with highly artificial input.Fixes IJPL-96392.
Produced by Air Automations. Name: Markdown library: fix the bug / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/cb6a17f6-e7a9-41a8-9611-7c95c4eb0141?run=e9cbf485-8272-43f8-9187-aab50412fdd2
🤖 Generated with Claude Code