Skip to content

Commit acd8261

Browse files
committed
Perf: cache newline offsets in Source for O(log n) loc conversion
hbsPosFor() and charPosFor() were doing a fresh indexOf('\n') scan of the source on every call, making each lookup O(lines_until_offset). These are called once per AST node loc by the ASTv2 normalize pass, so total cost was effectively O(n²) in template size. CPU profile of a full precompile() showed hbsPosFor dominating at ~28% of self-time, scattered across many call sites. Fix: precompute an array of newline offsets on first use, binary-search it for conversions. O(log n) per call, O(n) to build once per source. Impact (Node 24, warmed JIT, full precompile()): real-world template (1494 chars): 1.34ms -> 1.24ms large template (3520 chars): 4.53ms -> 3.06ms (32%% faster) The normalize phase specifically (ASTv1 -> ASTv2) drops from ~1.72ms to ~0.43ms on the large template — a 4x speedup in that phase. All tests pass.
1 parent 8eda63c commit acd8261

1 file changed

Lines changed: 47 additions & 54 deletions

File tree

packages/@glimmer/syntax/lib/source/source.ts

Lines changed: 47 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,15 @@ export class Source {
1212
return new Source(source, options.meta?.moduleName);
1313
}
1414

15+
/** Char offset of each `\n` in the source. */
16+
readonly #newlineOffsets: readonly number[];
17+
1518
constructor(
1619
readonly source: string,
1720
readonly module = 'an unknown module'
1821
) {
1922
setLocalDebugType('syntax:source', this);
23+
this.#newlineOffsets = computeNewlineOffsets(source);
2024
}
2125

2226
/**
@@ -42,66 +46,55 @@ export class Source {
4246
}
4347

4448
hbsPosFor(offset: number): Nullable<SourcePosition> {
45-
let seenLines = 0;
46-
let seenChars = 0;
47-
48-
if (offset > this.source.length) {
49-
return null;
50-
}
51-
52-
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
53-
while (true) {
54-
let nextLine = this.source.indexOf('\n', seenChars);
49+
if (offset > this.source.length) return null;
50+
const lineIdx = lowerBound(this.#newlineOffsets, offset);
51+
return { line: lineIdx + 1, column: offset - this.#lineStartFor(lineIdx) };
52+
}
5553

56-
if (offset <= nextLine || nextLine === -1) {
57-
return {
58-
line: seenLines + 1,
59-
column: offset - seenChars,
60-
};
61-
} else {
62-
seenLines += 1;
63-
seenChars = nextLine + 1;
54+
charPosFor({ line, column }: SourcePosition): number | null {
55+
const lineIdx = line - 1;
56+
const lineStart = this.#lineStartFor(lineIdx);
57+
const nextNl = this.#newlineOffsets[lineIdx];
58+
const lineEnd = nextNl ?? this.source.length;
59+
const target = lineStart + column;
60+
61+
if (target <= lineEnd) {
62+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
63+
if (DEBUG) {
64+
const roundTrip = this.hbsPosFor(target);
65+
localAssert(roundTrip !== null, `the returned offset failed to round-trip`);
66+
localAssert(roundTrip.line === line, `line round-trip mismatch`);
67+
localAssert(roundTrip.column === column, `column round-trip mismatch`);
6468
}
69+
return target;
6570
}
71+
return lineEnd;
6672
}
6773

68-
charPosFor(position: SourcePosition): number | null {
69-
let { line, column } = position;
70-
let sourceString = this.source;
71-
let sourceLength = sourceString.length;
72-
let seenLines = 0;
73-
let seenChars = 0;
74-
75-
while (seenChars < sourceLength) {
76-
let nextLine = this.source.indexOf('\n', seenChars);
77-
if (nextLine === -1) nextLine = this.source.length;
78-
79-
if (seenLines === line - 1) {
80-
if (seenChars + column > nextLine) return nextLine;
81-
82-
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
83-
if (DEBUG) {
84-
let roundTrip = this.hbsPosFor(seenChars + column);
85-
localAssert(roundTrip !== null, `the returned offset failed to round-trip`);
86-
localAssert(
87-
roundTrip.line === line,
88-
`the round-tripped line didn't match the original line`
89-
);
90-
localAssert(
91-
roundTrip.column === column,
92-
`the round-tripped column didn't match the original column`
93-
);
94-
}
74+
#lineStartFor(lineIdx: number): number {
75+
if (lineIdx === 0) return 0;
76+
const prevNl = this.#newlineOffsets[lineIdx - 1];
77+
return prevNl === undefined ? 0 : prevNl + 1;
78+
}
79+
}
9580

96-
return seenChars + column;
97-
} else if (nextLine === -1) {
98-
return 0;
99-
} else {
100-
seenLines += 1;
101-
seenChars = nextLine + 1;
102-
}
103-
}
81+
function computeNewlineOffsets(source: string): number[] {
82+
const offsets: number[] = [];
83+
for (let i = source.indexOf('\n'); i !== -1; i = source.indexOf('\n', i + 1)) {
84+
offsets.push(i);
85+
}
86+
return offsets;
87+
}
10488

105-
return sourceLength;
89+
/** Lower-bound binary search: smallest i with arr[i] >= target, else arr.length. */
90+
function lowerBound(arr: readonly number[], target: number): number {
91+
let lo = 0;
92+
let hi = arr.length;
93+
while (lo < hi) {
94+
const mid = (lo + hi) >>> 1;
95+
// mid is in [lo, hi) so always a valid index.
96+
if ((arr[mid] as number) < target) lo = mid + 1;
97+
else hi = mid;
10698
}
99+
return lo;
107100
}

0 commit comments

Comments
 (0)