-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix: inlineTokens rebuilds the reflink-mask preamble per call #4040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
3f0a750
abb5d89
ea3b3f0
ea56785
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { marked } from '../../lib/marked.esm.js'; | ||
| import { describe, it } from 'node:test'; | ||
| import assert from 'node:assert'; | ||
|
|
||
| /** | ||
| * Regression: many reference definitions + references must not be quadratic. | ||
| * | ||
| * Lexer.inlineTokens rebuilt its reflink-masking preamble (Object.keys over | ||
| * every link definition, then a reflinkSearch replace) on *every* call — | ||
| * including recursive link-text calls whose text cannot contain a reflink. | ||
| * With n defs and n refs that is O(n²) allocation churn (measured exponent | ||
| * ~3: 24s at n=13000). The fix skips the masking block when the source has | ||
| * no '[' (reflinkSearch cannot match without one) and uses a Set lookup. | ||
| */ | ||
| function footnoteShape(n) { | ||
| let refs = ''; | ||
| const defs = []; | ||
| for (let i = 0; i < n; i++) { | ||
| refs += `[^${i}] `; | ||
| defs.push(`[^${i}]: x`); | ||
| } | ||
| return refs + '\n\n' + defs.join('\n'); | ||
| } | ||
|
|
||
| function parseSeconds(text) { | ||
| marked.parse('warmup'); | ||
| const t0 = process.hrtime.bigint(); | ||
| marked.parse(text); | ||
| return Number(process.hrtime.bigint() - t0) / 1e9; | ||
| } | ||
|
|
||
| describe('inlineTokens masking scaling', () => { | ||
| it('stays near-linear over a doubling ladder', () => { | ||
| const t1 = parseSeconds(footnoteShape(2000)); | ||
| const t2 = parseSeconds(footnoteShape(4000)); | ||
| // pre-fix this ratio is ~5+ (exponent > 2); linear growth is ~2. | ||
| assert.ok( | ||
| t2 < Math.max(t1 * 3.5, 0.5), | ||
| `superlinear growth suspected: ${t1.toFixed(3)}s -> ${t2.toFixed(3)}s`, | ||
| ); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — replaced the absolute-time fallback with a direct ratio assertion plus an epsilon floor ( |
||
| }); | ||
|
|
||
| it('renders reference links identically to the unmasked path', () => { | ||
| assert.strictEqual( | ||
| marked.parse('[a][b]\n\n[b]: /url "t"'), | ||
| '<p><a href="/url" title="t">a</a></p>\n', | ||
| ); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create a test in test/specs/redos instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done — moved the regression coverage to test/specs/redos as quadratic_footnote_defs.cjs (13000 footnote-shaped defs + refs; ~24s pre-fix, ~0.07s post-fix) and dropped the unit test file. Re-verified before push: full suite green (npm test) and the scaling repro reports exponent 1.151.