From d10e1c7ffa7dfd8d8a69b831170a877e9144587d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 6 Oct 2026 21:31:24 -0700 Subject: [PATCH 1/2] perf(mobile): highlight source files in small batches that keep grammar state Opening a large source file tokenized it in 200-line batches. On a 3.2k-line file each batch ran 25-260ms on the JS thread, and batches restarted from an empty grammar state, so a comment or template string crossing a boundary lost its colors. Batches are now capped at 2,000 characters and resume from the previous batch's grammar state, matching the native review diff highlighter. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../review/shikiReviewHighlighter.test.ts | 18 +++++ .../features/review/shikiReviewHighlighter.ts | 71 ++++++++++--------- 2 files changed, 55 insertions(+), 34 deletions(-) diff --git a/apps/mobile/src/features/review/shikiReviewHighlighter.test.ts b/apps/mobile/src/features/review/shikiReviewHighlighter.test.ts index cfb28051cb12..dc8de530001c 100644 --- a/apps/mobile/src/features/review/shikiReviewHighlighter.test.ts +++ b/apps/mobile/src/features/review/shikiReviewHighlighter.test.ts @@ -27,6 +27,24 @@ describe("highlightSourceFile", () => { ); }); + it("keeps colors for a block comment that spans highlight batches", async () => { + // Long enough that the comment body crosses at least one batch boundary. + const body = Array.from({ length: 400 }, (_, index) => `const insideComment${index} = 1;`); + const highlighted = await highlightSourceFile({ + path: "example.ts", + contents: ["/*", ...body, "*/", "const after = 2;"].join("\n"), + theme: "dark", + }); + + const commentColors = new Set( + highlighted.slice(1, -2).flatMap((line) => line.map((token) => token.color)), + ); + expect(commentColors.size).toBe(1); + expect(highlighted.at(-1)?.map((token) => token.color)).not.toEqual( + highlighted[1]?.map((token) => token.color), + ); + }); + it("falls back to plain tokens for very long lines", async () => { const longLine = `const value = "${"a".repeat(1_100)}";`; diff --git a/apps/mobile/src/features/review/shikiReviewHighlighter.ts b/apps/mobile/src/features/review/shikiReviewHighlighter.ts index 9b69c8d552ea..63ee250caf6c 100644 --- a/apps/mobile/src/features/review/shikiReviewHighlighter.ts +++ b/apps/mobile/src/features/review/shikiReviewHighlighter.ts @@ -1,4 +1,4 @@ -import { createHighlighterCore, type HighlighterCore } from "@shikijs/core"; +import { createHighlighterCore, type GrammarState, type HighlighterCore } from "@shikijs/core"; import { createJavaScriptRegexEngine } from "@shikijs/engine-javascript"; import bashLanguage from "@shikijs/langs/bash"; import javascriptLanguage from "@shikijs/langs/javascript"; @@ -50,7 +50,9 @@ const REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE = resolveReviewHighlighterEnginePrefe REVIEW_HIGHLIGHTER_ENGINE_ENV_VALUE, ); const REVIEW_HIGHLIGHT_CHUNK_LINE_THRESHOLD = 8; -const REVIEW_HIGHLIGHT_CHUNK_SIZE = 200; +// Bounds each tokenizing task by characters, not lines, so long-line files +// still yield to touches and renders between short batches. +const REVIEW_HIGHLIGHT_CHUNK_CHARACTERS = 2_000; const REVIEW_TOKENIZE_MAX_LINE_LENGTH = 1_000; const REVIEW_INITIAL_LANGUAGE_MODULES = [ bashLanguage, @@ -513,46 +515,47 @@ async function highlightLines( const highlighter = await getHighlighter(); const sourceLines = code.split("\n"); const highlightedLines: Array> = []; - const shortLineBatch: string[] = []; - - const flushShortLineBatch = async (): Promise => { - if (shortLineBatch.length === 0) { - return; - } - - const tokenLines = highlighter.codeToTokensBase(shortLineBatch.join("\n"), { - lang: language, - theme, - }); - highlightedLines.push(...normalizeHighlightedLines(tokenLines)); - shortLineBatch.length = 0; - }; - - for (let lineIndex = 0; lineIndex < sourceLines.length; lineIndex += 1) { - const line = sourceLines[lineIndex] ?? ""; - - if (line.length > REVIEW_TOKENIZE_MAX_LINE_LENGTH) { - await flushShortLineBatch(); - highlightedLines.push([{ content: line, color: null, fontStyle: null }]); + // Batches resume from the previous batch's grammar state, so a comment or + // template string that spans a batch boundary keeps its colors. + let grammarState: GrammarState | undefined; + let start = 0; + + while (start < sourceLines.length) { + // A skipped line leaves its ending state unknown; resume from a fresh state. + if (sourceLines[start]!.length > REVIEW_TOKENIZE_MAX_LINE_LENGTH) { + highlightedLines.push([{ content: sourceLines[start]!, color: null, fontStyle: null }]); + grammarState = undefined; + start += 1; } else { - shortLineBatch.push(line); - } + let end = start; + let characters = 0; + while (end < sourceLines.length) { + const length = sourceLines[end]!.length; + if ( + length > REVIEW_TOKENIZE_MAX_LINE_LENGTH || + (end > start && characters + length + 1 > REVIEW_HIGHLIGHT_CHUNK_CHARACTERS) + ) { + break; + } + characters += length + (end > start ? 1 : 0); + end += 1; + } - if (shortLineBatch.length >= REVIEW_HIGHLIGHT_CHUNK_SIZE) { - await flushShortLineBatch(); + const tokenLines = highlighter.codeToTokensBase(sourceLines.slice(start, end).join("\n"), { + lang: language, + theme, + grammarState, + }); + grammarState = highlighter.getLastGrammarState(tokenLines); + highlightedLines.push(...normalizeHighlightedLines(tokenLines)); + start = end; } - if ( - sourceLines.length > REVIEW_HIGHLIGHT_CHUNK_LINE_THRESHOLD && - lineIndex + 1 < sourceLines.length && - (shortLineBatch.length === 0 || line.length > REVIEW_TOKENIZE_MAX_LINE_LENGTH) - ) { + if (sourceLines.length > REVIEW_HIGHLIGHT_CHUNK_LINE_THRESHOLD && start < sourceLines.length) { await waitForNextFrame(); } } - await flushShortLineBatch(); - return highlightedLines; } From 4902bc3b0b8b2ea7e90873cd75e22d7cfc07c106 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 6 Oct 2026 21:45:20 -0700 Subject: [PATCH 2/2] perf(mobile): run several highlight batches per frame Yielding after every 2,000-character batch kept each JS task short but waited for a frame after each one, which about doubled the time until a large file got its colors. Batches now run back to back until about 16ms of work has passed, then yield once. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/features/review/shikiReviewHighlighter.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/mobile/src/features/review/shikiReviewHighlighter.ts b/apps/mobile/src/features/review/shikiReviewHighlighter.ts index 63ee250caf6c..b0ab139d0d27 100644 --- a/apps/mobile/src/features/review/shikiReviewHighlighter.ts +++ b/apps/mobile/src/features/review/shikiReviewHighlighter.ts @@ -50,9 +50,12 @@ const REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE = resolveReviewHighlighterEnginePrefe REVIEW_HIGHLIGHTER_ENGINE_ENV_VALUE, ); const REVIEW_HIGHLIGHT_CHUNK_LINE_THRESHOLD = 8; -// Bounds each tokenizing task by characters, not lines, so long-line files +// Bounds each tokenizing call by characters, not lines, so long-line files // still yield to touches and renders between short batches. const REVIEW_HIGHLIGHT_CHUNK_CHARACTERS = 2_000; +// A yield waits for the next frame, so batches run back to back until about one +// frame of work has passed instead of yielding after every batch. +const REVIEW_HIGHLIGHT_YIELD_AFTER_MS = 16; const REVIEW_TOKENIZE_MAX_LINE_LENGTH = 1_000; const REVIEW_INITIAL_LANGUAGE_MODULES = [ bashLanguage, @@ -519,6 +522,7 @@ async function highlightLines( // template string that spans a batch boundary keeps its colors. let grammarState: GrammarState | undefined; let start = 0; + let sliceStartedAt = performance.now(); while (start < sourceLines.length) { // A skipped line leaves its ending state unknown; resume from a fresh state. @@ -551,8 +555,13 @@ async function highlightLines( start = end; } - if (sourceLines.length > REVIEW_HIGHLIGHT_CHUNK_LINE_THRESHOLD && start < sourceLines.length) { + if ( + sourceLines.length > REVIEW_HIGHLIGHT_CHUNK_LINE_THRESHOLD && + start < sourceLines.length && + performance.now() - sliceStartedAt >= REVIEW_HIGHLIGHT_YIELD_AFTER_MS + ) { await waitForNextFrame(); + sliceStartedAt = performance.now(); } }