Skip to content

[repo-assist] perf: avoid node.children array allocation in hot AST traversal loops - #554

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
repo-assist/perf-avoid-node-children-array-20260820-cd656705152df40b
Draft

[repo-assist] perf: avoid node.children array allocation in hot AST traversal loops#554
github-actions[bot] wants to merge 1 commit into
mainfrom
repo-assist/perf-avoid-node-children-array-20260820-cd656705152df40b

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist, an AI assistant. See .github/workflows/repo-assist.md for details.

What

Tree-sitter's node.children getter materializes a new JS array on every access rather than returning a cached property. The main visit()/child-iteration loops in goAnalyzer.ts, javaAnalyzer.ts, jsLikeAnalyzer.ts (shared by JS/TS/TSX), pythonAnalyzer.ts, rustAnalyzer.ts, and two remaining loops in csharpAnalyzer.ts used for (const child of node.children). Since these visit() functions recurse over every node in the AST, this allocated one throwaway array per node visited across the whole file - the hottest path in the extension.

csharpAnalyzer.ts already applied the fix (with an explanatory comment) to most of its own loops; this PR extends the same already-proven pattern consistently to the remaining loops in that file and to all the other language analyzers:

// before
for (const child of node.children) {
  visit(child);
}

// after
for (let i = 0; i < node.childCount; i++) {
  const child = node.child(i)!;
  visit(child);
}

Why this is safe

  • Purely a traversal-mechanics change: iterates the identical set of children in the identical order, just without allocating an intermediate array.
  • No output/behavior change - confirmed by the full unit test suite passing unchanged.
  • Mirrors a pattern already reviewed and merged in this codebase (see comments in csharpAnalyzer.ts around the childCount/child(i) usage).

Test Status

  • npm run compile: clean
  • npm run lint: clean
  • npm run test:unit (mocha + c8 coverage): 225 passing, coverage unchanged (83.84% stmts / 93.66% branch / 96.19% funcs / 83.84% lines - matches baseline, no regression)
  • npm test (full vscode-test suite): not run - blocked by sandbox network restrictions (VS Code download), as documented in repo instructions; this is expected in this environment.

Closes no issue - proactive Task 8 (Performance Improvements) engineering-quality PR.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by 🌈 Repo Assist, see workflow run. Learn more.
Comment /repo-assist to run again

Add this agentic workflow to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@42c2ab5b4e4c9273534c39259b2e0df7f20f07e9

Tree-sitter's node.children getter materializes a new JS array on every
access rather than returning a cached property. The main visit()/traversal
loops in goAnalyzer, javaAnalyzer, jsLikeAnalyzer (used by JS/TS/TSX),
pythonAnalyzer, rustAnalyzer, and two remaining loops in csharpAnalyzer ran
this allocation on every single AST node visited during analysis - the
hottest path in the extension.

csharpAnalyzer.ts already used the childCount/child(i) pattern in most
places with a comment explaining why; this change applies the same
already-proven, behavior-preserving pattern consistently to the remaining
loops across all language analyzers, eliminating one throwaway array
allocation per node visited.

No behavioral change: iterates the same children in the same order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants