fix(app): stop quadratic diff text measurement freezing the renderer
Both advance-measurement paths measured a line by re-measuring a growing prefix per grapheme, so one n-grapheme line shaped n^2/2 characters. The web canvas measurer and the model fallback each carried a copy, and the native Skia branch had the same shape via getRectsForRange(0, end) per grapheme. Nothing caps line length client-side, so one long generated line in a diff pinned the renderer main thread for minutes and the app read as frozen while compositor animations kept running. Advance measurement now has one owner in text-measurement.ts: 64-grapheme chunks measured against an accumulated base, with chunk boundaries snapped to shaping-safe seams so ligatures and contextual scripts keep their exact prefix-measured widths (the xfi/Arabic contract in model.test.ts is unchanged). A 1.2M-char line drops from a ~331-minute projection to under a second; measurement-cost.test.ts pins the linear shaped-character budget so the quadratic cannot come back.
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
import type { ParsedDiffFile } from "@getpaseo/protocol/messages";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildDiffDocumentModel } from "./model";
|
||||
import type { BuildDiffDocumentModelInput, TextMeasurer } from "./types";
|
||||
|
||||
/**
|
||||
* A text backend that can only measure a whole string costs O(string length)
|
||||
* per call, so measuring cumulative advances by re-shaping a growing prefix
|
||||
* shapes n^2/2 characters for one line of n characters. That froze the renderer
|
||||
* for ~10s on a 42-line CHANGELOG.md diff and for over 15 minutes on a diff
|
||||
* containing a single 1.2M-character line.
|
||||
*
|
||||
* Cost is counted in *shaped characters* rather than calls, because that is the
|
||||
* quantity that maps to renderer CPU time. The budgets below are the linear
|
||||
* contract of the shared advance measurement in `text-measurement.ts`: work
|
||||
* stays proportional to the text, with a constant bounded by the shaped-run cap.
|
||||
*/
|
||||
const SHAPED_RUN_LIMIT = 64;
|
||||
|
||||
function countingMeasurer(): { stats: { shapedCharacters: number }; measurer: TextMeasurer } {
|
||||
const stats = { shapedCharacters: 0 };
|
||||
return {
|
||||
stats,
|
||||
measurer: {
|
||||
measure(text) {
|
||||
stats.shapedCharacters += text.length;
|
||||
return text.length * 10;
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/** Ligature-bearing source, so measurement takes the shaping path rather than the cache. */
|
||||
function codeLine(length: number): string {
|
||||
const unit = "const value = (a, b) => a === b ? 1 : 0; ";
|
||||
return unit.repeat(Math.ceil(length / unit.length)).slice(0, length);
|
||||
}
|
||||
|
||||
function diffFile(lines: readonly string[]): ParsedDiffFile {
|
||||
return {
|
||||
path: "src/a.ts",
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: lines.length,
|
||||
deletions: 0,
|
||||
hunks: [
|
||||
{
|
||||
oldStart: 1,
|
||||
oldCount: 0,
|
||||
newStart: 1,
|
||||
newCount: lines.length,
|
||||
lines: lines.map((content) => ({ type: "add" as const, content })),
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function shapedCharactersFor(options: { lineCount: number; lineLength: number }): number {
|
||||
const { stats, measurer } = countingMeasurer();
|
||||
const line = codeLine(options.lineLength);
|
||||
const input: BuildDiffDocumentModelInput = {
|
||||
files: [diffFile(Array.from({ length: options.lineCount }, () => line))],
|
||||
collapsedFilePaths: new Set(),
|
||||
layout: "unified",
|
||||
wrapLines: false,
|
||||
viewportWidth: 1200,
|
||||
typography: { family: "monospace", size: 12, lineHeight: 18 },
|
||||
measureText: measurer,
|
||||
palette: {
|
||||
surface: "#000",
|
||||
headerSurface: "#111",
|
||||
border: "#222",
|
||||
foreground: "#fff",
|
||||
foregroundMuted: "#aaa",
|
||||
addition: "green",
|
||||
deletion: "red",
|
||||
additionBackground: "#010",
|
||||
deletionBackground: "#100",
|
||||
emptyBackground: "#111",
|
||||
selection: "blue",
|
||||
syntax: {},
|
||||
},
|
||||
labels: { binary: "Binary", tooLarge: "Too large" },
|
||||
};
|
||||
buildDiffDocumentModel(input);
|
||||
return stats.shapedCharacters;
|
||||
}
|
||||
|
||||
describe("diff document measurement cost", () => {
|
||||
it("shapes a bounded number of characters per character of text", () => {
|
||||
// The shape of the CHANGELOG.md working diff that froze the renderer: 42
|
||||
// lines, longest 597 characters.
|
||||
const lineCount = 42;
|
||||
const lineLength = 597;
|
||||
const shaped = shapedCharactersFor({ lineCount, lineLength });
|
||||
expect(shaped).toBeLessThanOrEqual(lineCount * lineLength * SHAPED_RUN_LIMIT);
|
||||
});
|
||||
|
||||
it("does not grow super-linearly with line length", () => {
|
||||
const short = shapedCharactersFor({ lineCount: 1, lineLength: 50 });
|
||||
const long = shapedCharactersFor({ lineCount: 1, lineLength: 800 });
|
||||
// 16x the characters must not cost more than ~16x the shaping work.
|
||||
expect(long / short).toBeLessThan(32);
|
||||
});
|
||||
|
||||
it("grows no faster than linearly with line count", () => {
|
||||
const ten = shapedCharactersFor({ lineCount: 10, lineLength: 100 });
|
||||
const hundred = shapedCharactersFor({ lineCount: 100, lineLength: 100 });
|
||||
expect(hundred).toBeLessThanOrEqual(ten * 10);
|
||||
});
|
||||
});
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
} from "@/utils/diff-layout";
|
||||
import { compactHighlightTokens } from "@/utils/diff-rendering";
|
||||
import { getInlineReviewThreadState, getSplitInlineReviewThreadState } from "@/review/geometry";
|
||||
import { advancesFor } from "./text-measurement";
|
||||
import type {
|
||||
BuildDiffDocumentModelInput,
|
||||
DiffCell,
|
||||
@@ -378,9 +379,7 @@ function measureGraphemeSlice(
|
||||
): { graphemes: DiffGrapheme[]; width: number } {
|
||||
const segments = graphemes.slice(startIndex, endIndex);
|
||||
const displayGraphemes = segments.map((segment) => segment.text);
|
||||
const advances = measureText.measureAdvances
|
||||
? measureText.measureAdvances(displayGraphemes)
|
||||
: measurePrefixAdvances(displayGraphemes, measureText.measure);
|
||||
const advances = advancesFor(measureText)(displayGraphemes);
|
||||
return {
|
||||
graphemes: segments.map((segment, index) => ({
|
||||
start: segment.start,
|
||||
@@ -392,17 +391,6 @@ function measureGraphemeSlice(
|
||||
};
|
||||
}
|
||||
|
||||
function measurePrefixAdvances(
|
||||
graphemes: readonly string[],
|
||||
measureText: (text: string) => number,
|
||||
): number[] {
|
||||
let prefix = "";
|
||||
return graphemes.map((grapheme) => {
|
||||
prefix += grapheme;
|
||||
return measureText(prefix);
|
||||
});
|
||||
}
|
||||
|
||||
export function fragmentTextForRange(fragment: DiffFragment, start: number, end: number): string {
|
||||
"worklet";
|
||||
return fragment.graphemes
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
} from "./model";
|
||||
import { paintWebViewport } from "./paint.web";
|
||||
import { hasPointerDragStarted } from "./pointer-gesture";
|
||||
import { createMeasuredAdvances } from "./text-measurement";
|
||||
import { retainDiffViewport } from "./viewport";
|
||||
import type {
|
||||
DiffHit,
|
||||
@@ -816,18 +817,11 @@ function createWebTextMeasurer(typography: DiffTypography): TextMeasurer {
|
||||
const canvas = document.createElement("canvas");
|
||||
const context = canvas.getContext("2d");
|
||||
if (!context) return { measure: () => 0 };
|
||||
return {
|
||||
measure(text, weight = "regular") {
|
||||
context.font = `${weight === "semibold" ? 600 : 400} ${typography.size}px ${typography.family}`;
|
||||
return context.measureText(text).width;
|
||||
},
|
||||
measureAdvances(graphemes) {
|
||||
context.font = `${typography.size}px ${typography.family}`;
|
||||
let prefix = "";
|
||||
return graphemes.map((grapheme) => {
|
||||
prefix += grapheme;
|
||||
return context.measureText(prefix).width;
|
||||
});
|
||||
},
|
||||
const measure = (text: string, weight: "regular" | "semibold" = "regular") => {
|
||||
context.font = `${weight === "semibold" ? 600 : 400} ${typography.size}px ${typography.family}`;
|
||||
return context.measureText(text).width;
|
||||
};
|
||||
// Canvas exposes no glyph coverage, so `requiresShaping` alone decides which
|
||||
// runs a shaper has to see -- there is no `glyphIds` to fall back on.
|
||||
return { measure, measureAdvances: createMeasuredAdvances((text) => measure(text)) };
|
||||
}
|
||||
|
||||
@@ -2,7 +2,8 @@ import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
createCachedAsciiTextMetrics,
|
||||
createFallbackAwareTextMeasurer,
|
||||
requiresNativeParagraph,
|
||||
createMeasuredAdvances,
|
||||
requiresShaping,
|
||||
type PrimaryTextFace,
|
||||
} from "./text-measurement";
|
||||
|
||||
@@ -33,11 +34,11 @@ describe.each(["ios", "android"])("%s native text measurement", (platform) => {
|
||||
|
||||
describe("native paragraph retention", () => {
|
||||
it("keeps ordinary code on the allocation-free font path", () => {
|
||||
expect(requiresNativeParagraph("const answer = value + 1;")).toBe(false);
|
||||
expect(requiresShaping("const answer = value + 1;")).toBe(false);
|
||||
});
|
||||
|
||||
it.each(["value => next", "中", "e\u0301", "👩💻", "مرحبا"])("retains shaping for %s", (text) => {
|
||||
expect(requiresNativeParagraph(text)).toBe(true);
|
||||
expect(requiresShaping(text)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -74,6 +75,65 @@ describe("cached ASCII text metrics", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("chunked advance measurement", () => {
|
||||
/**
|
||||
* A face where every character is 10 wide, except that each ligature pair
|
||||
* fuses into a single 12-wide glyph. Splitting a pair across two shaped runs
|
||||
* would measure it as 20 and drift every advance after it.
|
||||
*/
|
||||
const LIGATURES = new Set(["=>", "==", "->"]);
|
||||
function shapingMeasure(text: string): number {
|
||||
let width = 0;
|
||||
for (let index = 0; index < text.length; ) {
|
||||
if (LIGATURES.has(text.slice(index, index + 2))) {
|
||||
width += 12;
|
||||
index += 2;
|
||||
} else {
|
||||
width += 10;
|
||||
index += 1;
|
||||
}
|
||||
}
|
||||
return width;
|
||||
}
|
||||
|
||||
function advancesOf(text: string): number[] {
|
||||
return createMeasuredAdvances(shapingMeasure)(Array.from(text));
|
||||
}
|
||||
|
||||
function expected(text: string): number[] {
|
||||
return Array.from(text, (_, index) => shapingMeasure(text.slice(0, index + 1)));
|
||||
}
|
||||
|
||||
it("measures short runs in one shaping context", () => {
|
||||
expect(advancesOf("a => b")).toEqual(expected("a => b"));
|
||||
});
|
||||
|
||||
it("keeps a ligature intact when it straddles a chunk boundary", () => {
|
||||
// Place the ligature so the unsnapped 64-grapheme boundary lands inside it.
|
||||
for (const offset of [62, 63, 64, 65]) {
|
||||
const text = `${"a".repeat(offset)}=>${"b".repeat(40)}`;
|
||||
expect(advancesOf(text), `ligature at ${offset}`).toEqual(expected(text));
|
||||
}
|
||||
});
|
||||
|
||||
it("measures long ligature-dense source without drift at any boundary", () => {
|
||||
const text = "const f = (a) => a == 1; ".repeat(20);
|
||||
expect(advancesOf(text)).toEqual(expected(text));
|
||||
});
|
||||
|
||||
it("stays on the additive path for long context-free source", () => {
|
||||
const measured: string[] = [];
|
||||
const advances = createMeasuredAdvances((text) => {
|
||||
measured.push(text);
|
||||
return text.length * 10;
|
||||
})(Array.from("abc ".repeat(64)));
|
||||
|
||||
expect(advances.at(-1)).toBe(256 * 10);
|
||||
// Each distinct grapheme measured once, never a prefix.
|
||||
expect(measured).toEqual(["a", "b", "c", " "]);
|
||||
});
|
||||
});
|
||||
|
||||
function primaryFaceWithout(...missing: string[]): PrimaryTextFace {
|
||||
return {
|
||||
glyphIds(text) {
|
||||
|
||||
@@ -12,13 +12,108 @@ export interface CachedAsciiTextMetrics {
|
||||
|
||||
const CODE_LIGATURE_CANDIDATE = /(?:===?|!==?|=>|<=|>=|->|::|\+\+|--)/;
|
||||
|
||||
export function requiresNativeParagraph(text: string): boolean {
|
||||
export function requiresShaping(text: string): boolean {
|
||||
return /[^\x20-\x7e\t]/u.test(text) || CODE_LIGATURE_CANDIDATE.test(text);
|
||||
}
|
||||
|
||||
/**
|
||||
* Longest run handed to a shaper in one piece.
|
||||
*
|
||||
* Every backend that can only measure a whole string costs O(run length) per
|
||||
* cumulative advance, so measuring a run of n graphemes shapes n^2/2 characters.
|
||||
* A diff line is not bounded by anything -- a minified bundle checked into the
|
||||
* repo produces single lines over a megabyte long, which shapes for hours. Runs
|
||||
* are therefore capped, which trades a bounded seam error at chunk boundaries
|
||||
* for linear cost.
|
||||
*/
|
||||
const SHAPED_RUN_LIMIT = 64;
|
||||
|
||||
/** How far a chunk boundary may slide back to land where shaping cannot join. */
|
||||
const BOUNDARY_SEARCH_LIMIT = 16;
|
||||
|
||||
/** Characters that code fonts fuse into ligature glyphs. */
|
||||
const LIGATURE_JOINING = /[=!<>+\-:&|*/~^?.]/;
|
||||
|
||||
export interface ShapedAdvances {
|
||||
/** True when `text` needs a real shaper: per-grapheme advances are not additive. */
|
||||
requiresShaping(text: string): boolean;
|
||||
/** Cumulative advances summed from per-grapheme widths. */
|
||||
measureAdditive(graphemes: readonly string[]): number[];
|
||||
/** Cumulative advances for the whole run, measured in one shaping context. */
|
||||
measureShaped(graphemes: readonly string[]): number[];
|
||||
}
|
||||
|
||||
export function createChunkedAdvanceMeasurer(
|
||||
advances: ShapedAdvances,
|
||||
): (graphemes: readonly string[]) => number[] {
|
||||
return (graphemes) => {
|
||||
const result = Array.from<number>({ length: graphemes.length });
|
||||
let base = 0;
|
||||
let start = 0;
|
||||
while (start < graphemes.length) {
|
||||
const end = chunkEnd(graphemes, start);
|
||||
const chunk = graphemes.slice(start, end);
|
||||
const measured = advances.requiresShaping(chunk.join(""))
|
||||
? advances.measureShaped(chunk)
|
||||
: advances.measureAdditive(chunk);
|
||||
for (let index = 0; index < chunk.length; index += 1) {
|
||||
result[start + index] = base + (measured[index] ?? 0);
|
||||
}
|
||||
base = result[end - 1] ?? base;
|
||||
start = end;
|
||||
}
|
||||
return result;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Advance measurement for a backend that can only measure a whole string, which
|
||||
* is every backend except a native paragraph shaper.
|
||||
*/
|
||||
export function createMeasuredAdvances(
|
||||
measure: (text: string) => number,
|
||||
): (graphemes: readonly string[]) => number[] {
|
||||
return createChunkedAdvanceMeasurer({
|
||||
requiresShaping,
|
||||
measureAdditive: createAdditiveAdvances(measure),
|
||||
measureShaped: (graphemes) => measurePrefixAdvances(graphemes, measure),
|
||||
});
|
||||
}
|
||||
|
||||
const measurerAdvances = new WeakMap<TextMeasurer, (graphemes: readonly string[]) => number[]>();
|
||||
|
||||
/** Advance measurement for `measurer`, defaulting backends that supply none. */
|
||||
export function advancesFor(measurer: TextMeasurer): (graphemes: readonly string[]) => number[] {
|
||||
const existing = measurerAdvances.get(measurer);
|
||||
if (existing) return existing;
|
||||
const resolved = measurer.measureAdvances
|
||||
? measurer.measureAdvances.bind(measurer)
|
||||
: createMeasuredAdvances((text) => measurer.measure(text));
|
||||
measurerAdvances.set(measurer, resolved);
|
||||
return resolved;
|
||||
}
|
||||
|
||||
/** Cumulative advances summed from per-grapheme widths, each measured once. */
|
||||
export function createAdditiveAdvances(
|
||||
measure: (text: string) => number,
|
||||
): (graphemes: readonly string[]) => number[] {
|
||||
const widths = new Map<string, number>();
|
||||
return (graphemes) => {
|
||||
let advance = 0;
|
||||
return graphemes.map((grapheme) => {
|
||||
let width = widths.get(grapheme);
|
||||
if (width === undefined) {
|
||||
width = measure(grapheme);
|
||||
widths.set(grapheme, width);
|
||||
}
|
||||
advance += width;
|
||||
return advance;
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
export function createCachedAsciiTextMetrics(primary: PrimaryTextFace): CachedAsciiTextMetrics {
|
||||
const glyphCoverage = new Map<string, boolean>();
|
||||
const widths = new Map<string, number>();
|
||||
return {
|
||||
hasEveryGlyph(text) {
|
||||
for (const character of text) {
|
||||
@@ -31,18 +126,7 @@ export function createCachedAsciiTextMetrics(primary: PrimaryTextFace): CachedAs
|
||||
}
|
||||
return true;
|
||||
},
|
||||
measureAdvances(graphemes) {
|
||||
let advance = 0;
|
||||
return graphemes.map((grapheme) => {
|
||||
let width = widths.get(grapheme);
|
||||
if (width === undefined) {
|
||||
width = primary.measure(grapheme);
|
||||
widths.set(grapheme, width);
|
||||
}
|
||||
advance += width;
|
||||
return advance;
|
||||
});
|
||||
},
|
||||
measureAdvances: createAdditiveAdvances((text) => primary.measure(text)),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -58,3 +142,34 @@ export function createFallbackAwareTextMeasurer(input: {
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function measurePrefixAdvances(
|
||||
graphemes: readonly string[],
|
||||
measure: (text: string) => number,
|
||||
): number[] {
|
||||
let prefix = "";
|
||||
return graphemes.map((grapheme) => {
|
||||
prefix += grapheme;
|
||||
return measure(prefix);
|
||||
});
|
||||
}
|
||||
|
||||
function chunkEnd(graphemes: readonly string[], start: number): number {
|
||||
const limit = start + SHAPED_RUN_LIMIT;
|
||||
if (limit >= graphemes.length) return graphemes.length;
|
||||
const floor = Math.max(start + 1, limit - BOUNDARY_SEARCH_LIMIT);
|
||||
for (let end = limit; end > floor; end -= 1) {
|
||||
if (splitsCleanly(graphemes[end - 1]) && splitsCleanly(graphemes[end])) return end;
|
||||
}
|
||||
return limit;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a boundary next to `grapheme` is one no shaper reaches across. Ligature
|
||||
* glyphs are fused from ASCII punctuation and complex scripts join within a word,
|
||||
* so a printable-ASCII grapheme carrying no ligature punctuation is a safe seam.
|
||||
*/
|
||||
function splitsCleanly(grapheme: string | undefined): boolean {
|
||||
if (grapheme === undefined) return true;
|
||||
return /^[\x20-\x7e]+$/.test(grapheme) && !LIGATURE_JOINING.test(grapheme);
|
||||
}
|
||||
|
||||
@@ -9,8 +9,9 @@ import { fragmentTextForRange } from "./model";
|
||||
import { codeTextColor } from "./palette";
|
||||
import {
|
||||
createCachedAsciiTextMetrics,
|
||||
createChunkedAdvanceMeasurer,
|
||||
createFallbackAwareTextMeasurer,
|
||||
requiresNativeParagraph,
|
||||
requiresShaping,
|
||||
type CachedAsciiTextMetrics,
|
||||
} from "./text-measurement";
|
||||
import type { DiffCell, DiffDocumentModel, DiffFragment, DiffPalette, TextMeasurer } from "./types";
|
||||
@@ -120,34 +121,35 @@ export function createNativeTextMeasurer(input: {
|
||||
});
|
||||
return {
|
||||
...fallbackAware,
|
||||
measureAdvances(graphemes) {
|
||||
const text = graphemes.join("");
|
||||
if (!requiresRetainedParagraph(text, asciiMetrics))
|
||||
return asciiMetrics.measureAdvances(graphemes);
|
||||
const paragraph = createParagraph({
|
||||
text,
|
||||
families,
|
||||
fontSize: input.fontSize,
|
||||
lineHeight: Math.round(input.fontSize * 1.5),
|
||||
color: Skia.Color("black"),
|
||||
});
|
||||
let end = 0;
|
||||
const advances = graphemes.map((grapheme) => {
|
||||
end += grapheme.length;
|
||||
const rectangles = paragraph.getRectsForRange(0, end);
|
||||
return rectangles.reduce(
|
||||
(right, rectangle) => Math.max(right, rectangle.x + rectangle.width),
|
||||
0,
|
||||
);
|
||||
});
|
||||
paragraph.dispose();
|
||||
return advances;
|
||||
},
|
||||
measureAdvances: createChunkedAdvanceMeasurer({
|
||||
requiresShaping: (text) => requiresRetainedParagraph(text, asciiMetrics),
|
||||
measureAdditive: (graphemes) => asciiMetrics.measureAdvances(graphemes),
|
||||
measureShaped(graphemes) {
|
||||
const paragraph = createParagraph({
|
||||
text: graphemes.join(""),
|
||||
families,
|
||||
fontSize: input.fontSize,
|
||||
lineHeight: Math.round(input.fontSize * 1.5),
|
||||
color: Skia.Color("black"),
|
||||
});
|
||||
let end = 0;
|
||||
const advances = graphemes.map((grapheme) => {
|
||||
end += grapheme.length;
|
||||
const rectangles = paragraph.getRectsForRange(0, end);
|
||||
return rectangles.reduce(
|
||||
(right, rectangle) => Math.max(right, rectangle.x + rectangle.width),
|
||||
0,
|
||||
);
|
||||
});
|
||||
paragraph.dispose();
|
||||
return advances;
|
||||
},
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
function requiresRetainedParagraph(text: string, asciiMetrics: CachedAsciiTextMetrics): boolean {
|
||||
return requiresNativeParagraph(text) || !asciiMetrics.hasEveryGlyph(text);
|
||||
return requiresShaping(text) || !asciiMetrics.hasEveryGlyph(text);
|
||||
}
|
||||
|
||||
function createFragmentParagraph(input: {
|
||||
|
||||
Reference in New Issue
Block a user