Skip to content

Avoid repeated token scans in liquid-html-parser - #1371

Merged
clauderic merged 3 commits into
Shopify:mainfrom
madmath:secfix/SECFIX-5-parser-seek
Oct 29, 2026
Merged

clauderic merged 3 commits into
Shopify:mainfrom
madmath:secfix/SECFIX-5-parser-seek

Conversation

@madmath

@madmath madmath commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

What are you adding in this PR?

Files with many HTML comments, <script>, <style> or <svg> elements, or raw Liquid tags like {% raw %} and {% comment %}, parsed in quadratic time: each of those nodes did work proportional to the whole file. There's one commit per cause:

  1. c8099ba. seekToSourceOffset runs once per HTML comment and raw HTML element, and it scanned the tokens from the first one every time. Tokens are in source order, so it binary-searches them now.
  2. 8ebb6aa. resliceTokensFrom runs once per raw Liquid tag, and it still scanned every token: once for a token straddling the end tag, and again to seek when none did. Only the first token ending past the end tag can straddle it, so the same binary search finds both.
  3. 5877ab7. A stray {{ or {% in a raw tag's body, as in {% raw %}{{ x{% endraw %}, runs past the end tag, and the reslice then re-tokenized the rest of the file, once per such tag. Now the tokenizer produces tokens as the parser reads them, so each part of the file is tokenized about once.
    • Inside Liquid output, the tokenizer jumps to the next }} with indexOf and remembers where it found it.
    • The reslice tracks the HTML context it resumes in (an open quoted attribute or tag) incrementally over the tokens it keeps.
    • Callers that read tokens by index go through hasToken and tokenAt, which tokenize up to that index. tokenCount is gone.

One parse of each document, Node 24.22.1:

Document Size main This PR
30,000 <!--x--> 234 KB 1,322 ms 21 ms
30,000 <script></script> 498 KB 3,165 ms 78 ms
30,000 {% comment %}x{% endcomment %} 878 KB 9,572 ms 71 ms
30,000 {% raw %}x{% endraw %} 644 KB 9,561 ms 65 ms
3,000 {% raw %}{{ x{% endraw %} 73 KB 1,817 ms 7 ms
3,000 {% comment %}{{ x{% endcomment %} 96 KB 2,093 ms 7 ms
3,000 {% raw %}{{ x{% endraw %}, then }} and 9,000 <a>b</a> 143 KB 22,719 ms 37 ms
1,500 <a href="{% raw %}{{ x{% endraw %}">{% raw %}{{ x{% endraw %}</a>, then }} and 9,000 <a>b</a> 165 KB 16,138 ms 34 ms

Typical files parse at the same speed as on main. Dawn (98 files) takes 0.98× main's time and Horizon (231 files) 1.00×, and repeated runs vary by about ±2%.

Compatibility

The PR answers the open questions about backward seeks and first-token matching:

  • Seeks. A seek still lands on the first token at or after the offset, whether it moves forward or backward.
    • One unit test checks every offset against a linear scan, including gaps and a zero-width token.
    • Another seeks behind the cursor.
  • Reslices. A reslice keeps the tokens before the boundary and resumes the tokenizer in the same HTML context as before.
  • Differential testing against main. These inputs gave the same tokens, the same trees from all four parser entry points, and the same errors on both:
    • 300,000 generated documents that put a stray {{ inside raw tags in every HTML context, with about 189,000 straddling reslices between them;
    • 100,000 fuzzed documents;
    • the 333 Liquid files of Dawn and Horizon.

Tests

The original description said that the added tests fail without the fix. That holds for the three tests that count work, and not for the others, which pin down behavior:

Test Checks On main
seekToSourceOffset moves to the first token at or after the offset The position for every offset Passes
seekToSourceOffset seeks backward A seek behind the cursor Passes
Successive forward seeks don't rescan consumed tokens Token reads over 4,096 seeks Fails: 8.4M reads, limit 131k
Successive reslices that need no re-tokenize don't rescan Token reads over 4,096 reslices Fails: 25M reads, limit 131k
Re-tokenizes only as far as the parser reads after a straddling raw tag The tokenizer's probes of the source over 200 straddling reslices Fails: 2.8M probes, limit 26k (this PR: about 4,000)
Spec case in specs/parser/html.yml with two HTML comments, a <script> and a <style> side by side The full tree, in every parser that runs the specs Passes

What's next? Any followup issues?

Nothing blocking. CI passed on 5877ab7: code quality, E2E, and the unit tests on Ubuntu and Windows with Node 22 and 24.

Tophatting

Run locally on Node 24.22.1:

  • pnpm build:ts and pnpm type-check pass.
  • pnpm exec vitest --run: 7,817 passed, 5 skipped.
  • pnpm --dir packages/prettier-plugin-liquid test --run and test:3 --run: 144 passed each.
  • Prettier checks on the changed files pass.

Before you deploy

  • I included a patch bump changeset

🤖 Generated with Claude Code

Preserve the first-token-at-or-after semantics for forward and backward seeks while using sorted token offsets for bounded lookup work.

Co-authored-by: River <river@shopify.com>
@madmath
madmath marked this pull request as ready for review October 29, 2026 15:15
@madmath
madmath requested a review from a team as a code owner October 29, 2026 15:15
@madmath

This comment was marked as resolved.

clauderic and others added 2 commits October 29, 2026 12:12
resliceTokensFrom runs once per raw Liquid tag, like {% raw %} or
{% comment %}, and still scanned every token: once for a token
straddling the end tag, and again, when none did, for the first token
after it. So parsing stayed quadratic in the number of those tags:
30,000 {% comment %}x{% endcomment %} took 6 s, and now take 34 ms.

Tokens are in source order and don't overlap, so the only token that
can straddle the end tag is the first one ending past it, and a binary
search finds it. When none straddles, the reslice seeks, which is the
binary search from the previous commit. Both are one helper now.

The two tests that parsed 2,000 comments or raw elements become a spec
case with four of them side by side. They passed before the fix too,
so what they checked was the tree, and the spec checks all of it, in
every parser that runs the specs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A stray {{ or {% in a raw tag's body, as in {% raw %}{{ x{% endraw %},
leaves the tokenizer inside Liquid output at the end tag. The token
there runs past it, so resliceTokensFrom re-tokenizes from the end tag
on, and it did so through the end of the file, once per such tag.
3,000 of them took 1.5 to 22 s, depending on what follows, and now
take 9 to 59 ms.

The tokenizer now produces tokens as the parser reads them, and the
next straddling tag replaces them again, so each part of the file is
tokenized about once. For that:

- createTokenizer restarts at an offset and fills on demand, and
  tokenize is that, run to the end;
- inside Liquid output, the tokenizer jumps to the next }} with
  indexOf, and remembers where it found one, since restarts can land
  inside the same unclosed {{ many times;
- the HTML context a reslice resumes in, an open quoted attribute or
  tag, is folded over the kept tokens incrementally, instead of from
  the first token each time.

Callers that read tokens by index go through hasToken and tokenAt,
which tokenize up to the index, and tokenCount is gone. The new test
counts the tokenizer's probes of the source across 200 straddling
reslices: about 4,000 now, 2.8 million before.

Whole files parse no slower: Dawn and Horizon take 0.96 to 1.0 times
as long. Across 350,000 generated documents with straddling raw tags,
200,000 fuzzed ones and the 333 theme files, the tokens, trees and
errors from all four parser entry points are the same as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@clauderic
clauderic merged commit bb33f49 into Shopify:main Oct 29, 2026
8 checks passed
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.

2 participants