Skip to content

API: cache SourceFile text to heavily improve performance#4414

Merged
andrewbranch merged 1 commit into
microsoft:mainfrom
JetBrains:api-cache-text
Jun 23, 2026
Merged

API: cache SourceFile text to heavily improve performance#4414
andrewbranch merged 1 commit into
microsoft:mainfrom
JetBrains:api-cache-text

Conversation

@piotrtomiak

Copy link
Copy Markdown
Contributor

When using getTokenPosOfNode on various nodes, each time SourceFile text is required causing huge performance issues. This is a minimal change to improve performance in such cases. I wonder though, whether such a cache shouldn't be on each Node. CC @andrewbranch

Copilot AI review requested due to automatic review settings June 23, 2026 18:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves performance in the native-preview remote AST layer by caching SourceFile.text so repeated calls (e.g. from getTokenPosOfNodegetStart/getText) don’t repeatedly decode the source text from the string table.

Changes:

  • Add a __cachedText field and cache logic to the generated RemoteNode.text getter for SyntaxKind.SourceFile.
  • Update the generator (_scripts/generate-encoder.ts) so the caching logic is consistently produced in node.generated.ts.

Reviewed changes

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

File Description
_scripts/generate-encoder.ts Updates code generation for RemoteNode.text to include a SourceFile text cache.
_packages/native-preview/src/api/node/node.generated.ts Generated output reflecting the new SourceFile text cache in RemoteNode.text.

Comment thread _scripts/generate-encoder.ts Outdated
Comment thread _packages/native-preview/src/api/node/node.generated.ts Outdated
@andrewbranch

Copy link
Copy Markdown
Member

This is a good optimization—could you put the cache on RemoteSourceFile instead of RemoteNode and override get text() there?

@andrewbranch andrewbranch added this pull request to the merge queue Jun 23, 2026
Merged via the queue into microsoft:main with commit c623b07 Jun 23, 2026
21 checks passed
@piotrtomiak piotrtomiak deleted the api-cache-text branch June 24, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants