[pull] develop from mermaid-js:develop - #232
Merged
Merged
Conversation
Two whitespace-related regex pathologies made deeply-indented flowcharts parse in quadratic time: - anyCommentRegex (used by diagram-type detection and preprocessing) re-scanned each indent run from every position because of its leading greedy \s*. Guarding where the run may start makes it linear, with byte-identical output (proven by regexes.spec.ts against the legacy pattern as an oracle, plus an O(n) assertion). - The jison flowchart lexer matched a single whitespace char per SPACE token; it now matches a whitespace run. The grammar only uses SPACE as a separator, so parsing is unchanged. A deeply-indented fixture that took ~1.4 s to parse now takes ~30 ms. Split out of #7872 per review feedback so it can land and bisect independently.
Fastdom is currently using UMD, and so uses AMD vs CommonJS based on whether there is a global `define` function available. Unfortunately, this breaks some setups, so we need an ESM version of this module. The easiest way of doing is to patch `fastdom`, but this means we need to bundle it, moving it to a `devDependency`.
Fixes a merge conflict in the Cypress IIFE file that git locally could solve, but GitHub couldn't.
Review feedback: the (?<=\S) lookbehind needs Safari 16.4 and the project floor is 15.4 — and since regexes.ts loads for every diagram, an engine without it dies with a module-load SyntaxError. Restore the exported anyCommentRegex to its released form byte-for-byte: it is published surface and consumers replace with '\n', so adopting the capture-group variant there would silently eat a character of their output. The fast path is a new stripAnyComments() instead, wrapping the (^|\S) capture pattern and its $1 replacement as one unit; detectType uses it, and the equivalence spec now proves it against the exported regex as the oracle. Changeset trimmed to a one-liner.
…efine-bug fix: bundle `fastdom` to fix issues with global `define`
CodeQL flagged two polynomial-regex findings on this PR and the CWE-1333
review flagged a third. All three are correct, and measurement says the
guarded pattern this branch introduced did not fix the problem it claimed
to: it changed the constant, not the exponent.
Doubling the input still quadrupled the work, on both shapes, before this
commit:
lines guarded released
200 1.0ms 38.2ms
400 4.0ms 151.3ms
800 15.1ms 591.1ms
1600 56.8ms 2301.8ms
Two inputs reach a quarter-second inside the default 50k maxTextSize,
against 0.1ms for ordinary diagram text of the same length:
('\n' + ' '.repeat(4)).repeat(10_000) all whitespace 256ms
'%%' + 'x%%'.repeat(16_000) no newline 339ms
No regex form fixes this. Every variant needs a `\s*` that crosses
newlines, and `/m` hands the engine a candidate start at each line, so an
all-whitespace document rescans the remaining run once per line. The
`(^|\S)` guard removes the candidates inside an indent but not the one at
each line start.
So this replaces the pattern with a forward scan. For each `%%` it takes
the line's newline as the end -- `.` never matches a newline, so the regex
ended at the same character -- and extends left over the preceding
whitespace, never past the previous match. Each character is visited at
most twice. Both worst cases drop to 0.02ms and 0.00ms.
`anyCommentRegex` stays exported byte-for-byte: it is published surface,
and it remains the equivalence oracle the scanner is proven against.
The old timing test asserted "under 200ms" on one fixed input, which a
quadratic implementation passes comfortably -- and did. It is now a
scaling test over three shapes: 4x the input must cost well under 16x.
Restoring the guarded regex fails it at 15.3x and 15.7x.
…arse perf(flowchart): avoid O(n²) parse cost on deeply-indented diagrams
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )