Split token streams in one pass - #4112
Merged
DmitrySharabin merged 2 commits intoSep 22, 2026
Merged
DmitrySharabin merged 2 commits into
DmitrySharabin merged 2 commits into
Conversation
DmitrySharabin
added this pull request to stack #4113
September 16, 2026 07:33
✅ Deploy Preview for dev-prismjs-com ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
DmitrySharabin
marked this pull request as draft
September 16, 2026 07:39
DmitrySharabin
force-pushed
the
claude/split-token-stream-perf
branch
from
September 16, 2026 10:45
487e7bb to
38aff0f
Compare
DmitrySharabin
marked this pull request as ready for review
September 16, 2026 16:00
LeaVerou
approved these changes
Sep 16, 2026
DmitrySharabin
removed this pull request from stack #4113
September 17, 2026 08:01
DmitrySharabin
changed the base branch from
claude/compassionate-brahmagupta-94vewq
to
v2
September 17, 2026 08:01
DmitrySharabin
force-pushed
the
claude/split-token-stream-perf
branch
from
September 17, 2026 08:01
38aff0f to
2b5ee8b
Compare
DmitrySharabin
changed the base branch from
v2
to
claude/inner-spec-detection
September 17, 2026 08:02
DmitrySharabin
added this pull request to stack #4115
September 17, 2026 08:02
This was referenced Sep 17, 2026
DmitrySharabin
removed this pull request from stack #4115
September 22, 2026 08:35
DmitrySharabin
changed the base branch from
claude/inner-spec-detection
to
claude/compassionate-brahmagupta-94vewq
September 22, 2026 08:35
`splitTokenStream` copied the remainder of the stream for every offset (`stream.slice(i)`, `[b, ...stream.slice(i + 1)]`), and `embed` passes one offset per selected run, i.e. one per diff line. A 12k-line `diff:javascript` took 3197ms; `--cpu-prof` put 80% of samples in `splitAt`. Walk the stream once instead, carrying the straddling item into the next segment. `diff:javascript`, this branch vs its base: 3k lines 189ms -> 103ms, 12k lines 3197ms -> 307ms, 48k lines 1290ms, i.e. linear again. Two details the one-pass form has to preserve, both easy to get wrong and both covered by new tests: - A zero-length item sitting exactly on an offset belongs to the segment that starts there, not the one that ends there. `splitAt` got this from checking `pos >= offset` before consuming the item; without the `offsets[next] === start` clause, `diff:markdown` drops the empty `code-block` token that markdown emits for an empty fenced block. - With no offsets there is nothing to split. That is the common path — `:text` is the default selector, so every templating language takes it — so it must not walk the stream measuring items. 200k random streams split both ways against the previous implementation: no divergences. `splitTokenStream` and `insertTokens` are public API via `shared.js` and had no direct tests; add them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test's note described a placeholder that no longer exists, so it could no longer fail for the reason it was written. The input still covers the embedding it was about: the CSS around a PHP block in a `style` attribute. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DmitrySharabin
changed the base branch from
claude/compassionate-brahmagupta-94vewq
to
v2
September 22, 2026 08:48
DmitrySharabin
force-pushed
the
claude/split-token-stream-perf
branch
from
September 22, 2026 08:48
2b5ee8b to
0b53870
Compare
DmitrySharabin
added a commit
that referenced
this pull request
Sep 22, 2026
`container.splice(start, n, ...segment)` in `embed` and `stream.splice(i, 1, ...parts)` in `insertTokens` pass every item as a call argument, which overflows the stack once a document has more inner tokens than the engine takes arguments — about 460 KB of markup through any templating language, where `v2` was fine. Both now go through `replaceRange()`, which truncates and pushes instead. A root-level `ignore` token is dissolved into the `:text` run, but the walk then took it again, so a selector naming both `:text` and that token fed its text to the inner language twice, and the second copy changed how the first one parsed. Also in `token-stream.js`: the two halves of a split token shared the original's alias array, so `addAlias` on one reached the other and the original; `insertTokens` silently sliced text away when offsets were not ascending, and now throws; and `splitTokenStream` no longer claims an isolation it does not give — segments share their unsplit tokens with the input. Tests for each, plus the `diff` behaviours nothing pinned: two selectors rather than one, the normal-diff `<`/`>` blocks, and the single-alias branch of `tokenMatches`. Follow-up to #4110 and #4112, whose code these are in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DmitrySharabin
changed the base branch from
v2
to
claude/compassionate-brahmagupta-94vewq
September 22, 2026 08:48
DmitrySharabin
added this pull request to stack #4124
September 22, 2026 08:48
DmitrySharabin
removed this pull request from stack #4124
September 22, 2026 08:49
DmitrySharabin
merged commit Sep 22, 2026
7e18205
into
claude/compassionate-brahmagupta-94vewq
20 checks passed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #4110, on top of its branch.
TL;DR
splitTokenStreamwas quadratic in the number of selected runs: a 12k-linediff:javascripttook 3197ms, now 307ms. One forward pass, no remainder copies.diff:markdowndrops a token), and the no-offsets fast path (the common one —:textis the default selector).splitTokenStream/insertTokensare public API viashared.jsand had no direct tests. Added.tests/languages/php!+css+css-extras/issue2008.testdescribed a placeholder Declarative$innerselectors and a token overlay helper #4110 deletes, so it could no longer fail for the reason it exists. Note re-pointed at what it now guards.The perf bug
splitTokenStreamcalledsplitAtonce per offset, andsplitAtcopied the rest of the stream each time (stream.slice(i),[b, ...stream.slice(i + 1)]).embedpasses one offset per run, i.e. one per diff line, so the copies dominate.Repro — N reps of a three-line block,
Prism.highlight(code, 'diff:javascript'):The 3k row is within noise — at that size the quadratic term is small, and a re-measurement on another machine gave 113 ms before and 120 ms after. The 12k row is the real signal, and it reproduces: 2790 ms before, 313 ms after.
node --cpu-profon the 12k case put 1829 of ~2300 samples insplitAt.The two cases that are easy to lose
A zero-length item sitting exactly on an offset belongs to the segment that starts there, not the one that ends there.
splitAtgot this for free by testingpos >= offsetbefore consuming the item; a strictoffsets[next] < posbound never fires for it. Markdown emits an emptycode-blocktoken for an empty fenced block, so the difference is visible:With no offsets there is nothing to split, and that is the path almost everything takes: only
src/languages/diff.jssetsselect, so php, django, handlebars, ejs, erb, smarty, liquid, latte, ftl, etlua and tt2 all reachsplitTokenStreamwithoffsets === []. It must not walk the stream measuring items to get there.200k random streams (nested tokens, aliases, empty items, offsets at 0 / total / past the end / duplicated) split by both implementations: no divergences.
Not fixed
splitItembuilds the halves withnew Token(type, content, alias), which leavesToken.lengthat 0. Inert today — split streams are never fed back to_matchGrammar, the only reader — and the field is documented@internalwith no guaranteed meaning, so threading real lengths through felt like more machinery than the payoff. Worth revisiting if a consumer migrated tosplitTokenStreamever re-tokenizes.Full suite green (10244 passing, 1 pre-existing pending), lint clean,
tscandtsc -p tests/tsconfig.jsonclean with no flags.🤖 Generated with Claude Code