6.4x speedup of AST materialization in JS API by auvred · Pull Request #2835 · microsoft/typescript-go

auvred · 2026-02-19T12:17:02Z

make id a getter
define index access getters on RemoteNodeList only for lists with >15 nodes
don't compute source file by walking up the tree in every RemoteNode constructor call
~~initialize all source file nodes in one hot loop~~
don't cache child nodes in dynamically created maps; refer to the source file's nodes array instead

On main, the materialize program.ts and materialize checker.ts benchmarks are incorrect because they don't reset the source file's _children cache.

To compare this PR with main, add file._children = undefined to the benchmark code, like so:

        .add("materialize checker.ts", async () => {
            file._children = undefined;
            file.forEachChild(function visit(node) {
                node.forEachChild(visit);
            });
        })

Benchmarks on main (incorrect):

┌─────────┬──────────────────────────┬─────────────────────┬─────────────────────┬────────────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name                │ Latency avg (ns)    │ Latency med (ns)    │ Throughput avg (ops/s) │ Throughput med (ops/s) │ Samples │
├─────────┼──────────────────────────┼─────────────────────┼─────────────────────┼────────────────────────┼────────────────────────┼─────────┤
│ 0       │ 'materialize program.ts' │ '1662097 ± 3.60%'   │ '1521254 ± 89851'   │ '634 ± 1.22%'          │ '657 ± 39'             │ 602     │
│ 1       │ 'materialize checker.ts' │ '72855176 ± 41.99%' │ '57846306 ± 200516' │ '16 ± 12.85%'          │ '17 ± 0'               │ 14      │
└─────────┴──────────────────────────┴─────────────────────┴─────────────────────┴────────────────────────┴────────────────────────┴─────────┘

Benchmarks on main (fixed):

┌─────────┬──────────────────────────┬─────────────────────┬───────────────────────┬────────────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name                │ Latency avg (ns)    │ Latency med (ns)      │ Throughput avg (ops/s) │ Throughput med (ops/s) │ Samples │
├─────────┼──────────────────────────┼─────────────────────┼───────────────────────┼────────────────────────┼────────────────────────┼─────────┤
│ 0       │ 'materialize program.ts' │ '16393483 ± 4.59%'  │ '14681675 ± 425336'   │ '63 ± 3.74%'           │ '68 ± 2'               │ 61      │
│ 1       │ 'materialize checker.ts' │ '250615850 ± 2.54%' │ '247339115 ± 4464518' │ '4 ± 2.46%'            │ '4 ± 0'                │ 10      │
└─────────┴──────────────────────────┴─────────────────────┴───────────────────────┴────────────────────────┴────────────────────────┴─────────┘

Benchmarks on this branch:

┌─────────┬──────────────────────────┬─────────────────────┬──────────────────────┬────────────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name                │ Latency avg (ns)    │ Latency med (ns)     │ Throughput avg (ops/s) │ Throughput med (ops/s) │ Samples │
├─────────┼──────────────────────────┼─────────────────────┼──────────────────────┼────────────────────────┼────────────────────────┼─────────┤
│ 0       │ 'materialize program.ts' │ '2553450 ± 2.69%'   │ '2396055 ± 30939'    │ '405 ± 1.24%'          │ '417 ± 5'              │ 392     │
│ 1       │ 'materialize checker.ts' │ '38940759 ± 10.35%' │ '39447118 ± 9466677' │ '27 ± 10.05%'          │ '25 ± 5'               │ 26      │
└─────────┴──────────────────────────┴─────────────────────┴──────────────────────┴────────────────────────┴────────────────────────┴─────────┘

parser.ts - 16.393ms -> 2.553ms (6.42x)
checker.ts - 250.616ms -> 38.941ms (6.43x)

_packages/api/test/api.sync.bench.ts

Copilot

Pull request overview

This PR optimizes the _packages/api JS client-side “remote AST” materialization path, aiming to significantly reduce the overhead of constructing RemoteSourceFile/RemoteNode trees, and updates the benchmarks to measure that materialization work more directly.

Changes:

Updates the sync/async benchmarks to “materialize” by reconstructing a fresh RemoteSourceFile from the underlying binary buffer instead of walking/caching children.
Refactors RemoteNode/RemoteNodeList to reduce per-node/list work (e.g., id becomes a getter, children are sourced from a pre-built nodes array, and index accessors are adjusted).
Adds a hot-loop initialization in RemoteSourceFile to pre-create node wrappers and avoid per-access map caching.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
`_packages/api/test/api.sync.bench.ts`	Adjusts materialization benchmark to reconstruct `RemoteSourceFile` from the binary backing data.
`_packages/api/test/api.async.bench.ts`	Same as sync benchmark, but for the async API bench harness.
`_packages/api/src/node.ts`	Implements new materialization strategy (pre-built `nodes` array), changes `id` to getters, and revises `RemoteNodeList` index accessor behavior.

_packages/api/test/api.async.bench.ts

Copilot · 2026-02-19T16:37:18Z

_packages/api/src/node.ts

+        return this.at(0);
+    }
+    get 1(): RemoteNode {
+        return this.at(1);
+    }
+    get 2(): RemoteNode {
+        return this.at(2);
+    }
+    get 3(): RemoteNode {
+        return this.at(3);
+    }
+    get 4(): RemoteNode {
+        return this.at(4);
+    }
+    get 5(): RemoteNode {
+        return this.at(5);
+    }
+    get 6(): RemoteNode {
+        return this.at(6);
+    }
+    get 7(): RemoteNode {
+        return this.at(7);


RemoteNodeList now defines fixed index getters (get 0() … get 15()) unconditionally. For lists with length < 16 (including length === 0), these getters will still call this.at(n) and can return an unrelated node instead of undefined, changing normal array out-of-bounds behavior (e.g. list[0] on an empty list). Add bounds checks in these getters (or only define them when n < this.length).

Suggested change

return this.at(0);

}

get 1(): RemoteNode {

return this.at(1);

}

get 2(): RemoteNode {

return this.at(2);

}

get 3(): RemoteNode {

return this.at(3);

}

get 4(): RemoteNode {

return this.at(4);

}

get 5(): RemoteNode {

return this.at(5);

}

get 6(): RemoteNode {

return this.at(6);

}

get 7(): RemoteNode {

return this.at(7);

return this.length > 0 ? this.at(0) : (undefined as any);

}

get 1(): RemoteNode {

return this.length > 1 ? this.at(1) : (undefined as any);

}

get 2(): RemoteNode {

return this.length > 2 ? this.at(2) : (undefined as any);

}

get 3(): RemoteNode {

return this.length > 3 ? this.at(3) : (undefined as any);

}

get 4(): RemoteNode {

return this.length > 4 ? this.at(4) : (undefined as any);

}

get 5(): RemoteNode {

return this.length > 5 ? this.at(5) : (undefined as any);

}

get 6(): RemoteNode {

return this.length > 6 ? this.at(6) : (undefined as any);

}

get 7(): RemoteNode {

return this.length > 7 ? this.at(7) : (undefined as any);

This is consistent with old approach.

typescript-go/_packages/api/src/node.ts

Lines 309 to 313 in 12e2936

Object.defineProperty(this, i, {

get() {

return this.at(i);

},

});

Fair, but it looks like at itself may have a bug for out-of-bounds indexes, which is now replicated onto accessors from 0–16. Would you mind adding the bounds check into at?

_packages/api/src/node.ts

_packages/api/test/api.sync.bench.ts

_packages/api/test/api.async.bench.ts

andrewbranch · 2026-02-19T16:47:55Z

initialize all source file nodes in one hot loop

This is a good way to make the benchmark faster, but that benchmark is intended to represent a worst-case scenario where someone touches every node. It was designed to be lazy on purpose. Imagine this being used in an auxiliary LSP server where someone registers some additional completions or refactors. Those handlers start with a document position, so in a file like checker.ts, a realistic implementation will likely only have to materialize a tiny, tiny fraction of all the nodes in the file during any given request.

auvred · 2026-02-19T16:58:15Z

This is a good way to make the benchmark faster, but that benchmark is intended to represent a worst-case scenario where someone touches every node. It was designed to be lazy on purpose. Imagine this being used in an auxiliary LSP server where someone registers some additional completions or refactors. Those handlers start with a document position, so in a file like checker.ts, a realistic implementation will likely only have to materialize a tiny, tiny fraction of all the nodes in the file during any given request.

I'm currently researching the performance of tsgo-powered JS lint rules. From my private evaluations (which I plan to opensource soon), lint rules tend to cumulatively touch up to 80% of all AST nodes (with a few exception rules that touch almost 100% of AST nodes).

Maybe we can add some kind of opt-in eager materialization for these usecases?

On main, lazy materialization of an already parsed AST is slower than parsing the source file from scratch with Strada; this doesn't seem right to me.

andrewbranch · 2026-02-19T17:04:56Z

Yeah, an option to do that would be great! It makes sense that a linter would want to look at the whole file.

auvred · 2026-02-19T17:58:26Z

Ok, it turns out that full AST traversal + lazy materialization is now only a few percent slower than bulk materialization + full AST traversal. Lint rules still have to do a full AST traversal, so there's no big win from bulk materialization. I reverted getOrCreateChildAtNodeIndex back to the lazy approach; it's still fast!

andrewbranch · 2026-02-19T19:06:54Z

_packages/api/src/node.ts

+        super(view, decoder, 1, undefined!, {} as unknown as RemoteSourceFile);
+        this.sourceFile = this;
+        this.nodes = Array((this.view.byteLength - this.offsetNodes) / NODE_LEN);
+        this.nodes[0] = new RemoteNode(this.view, this.decoder, 0, this, this);


Is this right? The 0-index slot is supposed to indicate an undefined node. I would have thought something would have broken 🤔

andrewbranch · 2026-02-19T19:25:08Z

_packages/api/src/node.ts

+    get id(): string {
+        const extendedDataOffset = this.offsetExtendedData + (this.data & NODE_EXTENDED_DATA_MASK);
+        return this.getString(this.view.getUint32(extendedDataOffset + 12, true));
+    }


I just merged something that will conflict with this in #2831, sorry. Should be a straightforward removal.

12e2936

auvred mentioned this pull request Feb 19, 2026

Fix infinite forEachChild loop on some nodes in JS API #2833

Open

auvred commented Feb 19, 2026

View reviewed changes

_packages/api/test/api.sync.bench.ts Outdated Show resolved Hide resolved

jakebailey requested review from andrewbranch and Copilot February 19, 2026 16:30

Copilot started reviewing on behalf of jakebailey February 19, 2026 16:31 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

auvred added 8 commits February 19, 2026 20:28

rewriterelativeimportextensions

8cffb58

cleanup

3749d9c

set correct node parent

7a9cd6e

fmt

5b11d39

not only

d2b9b01

public view & decoder

da71a22

lazy materialization!

19b9f66

fmt

433953d

andrewbranch reviewed Feb 19, 2026

View reviewed changes

	Object.defineProperty(this, i, {
	get() {
	return this.at(i);
	},
	});

Conversation

auvred commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

auvred Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

andrewbranch Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewbranch commented Feb 19, 2026

Uh oh!

auvred commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewbranch commented Feb 19, 2026

Uh oh!

auvred commented Feb 19, 2026

Uh oh!

andrewbranch Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

andrewbranch Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

auvred commented Feb 19, 2026 •

edited

Loading

auvred commented Feb 19, 2026 •

edited

Loading