Skip to content

Reduce path-mapping cache memory usage for projects with many paths - #63998

Merged
Andrew Branch (andrewbranch) merged 1 commit into
microsoft:mainfrom
auvred:reduce-path-pattern-cache-memory
Aug 26, 2026
Merged

Reduce path-mapping cache memory usage for projects with many paths#63998
Andrew Branch (andrewbranch) merged 1 commit into
microsoft:mainfrom
auvred:reduce-path-pattern-cache-memory

Conversation

@auvred

@auvred auvred commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #63997

This PR moves parsed patterns cache from resolutionState to caches (embedded by Resolver) so that different module resolutions can reuse already parsed patterns. As far as I understand, these patterns are immutable, so it's safe to cache them here.


Before: handled method 'textDocument/references' (22) in 2.518777125s

image

After: handled method 'textDocument/references' (23) in 1.461889542s

image

Copilot AI balanced review requested due to automatic review settings August 25, 2026 08:07
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Aug 25, 2026

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

Reduces path-mapping memory usage by sharing parsed patterns across module resolutions.

Changes:

  • Adds a resolver-level cache keyed by path mappings.
  • Corrects allocation sizing for parsed patterns and exact matches.

Reviewed changes

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

File Description
tsc/internal/module/cache.go Adds the shared parsed-pattern cache.
tsc/internal/module/resolver.go Uses the cache and improves parsing allocations.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread tsc/internal/module/cache.go
}
}
}
numMatchables := pathMappings.Size() - numPatterns

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I accidentally noticed that numPatterns is used for patterns (StarIndex != -1), but here it was counted as the number paths of StarIndex == -1.

Also, numMatchables can be smaller than pathMappings.Size() - numPatterns since some patterns may be invalid, so it's better to explicitly count number of valid patterns with StarIndex == -1.

@DanielRosenwasser

Copy link
Copy Markdown
Member

Linking back to microsoft/typescript-go#371 for the sake of context/continuity.

@DanielRosenwasser

Copy link
Copy Markdown
Member

I'm familiar with the core of all of this, but how exactly does this come up? Is it that multiple threads can do the same independent work on resolution? Or is it something around multi-project setups?

@jakebailey Jake Bailey (jakebailey) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is good?

@github-project-automation github-project-automation Bot moved this from Not started to Needs merge in PR Backlog Aug 25, 2026
@DanielRosenwasser

Copy link
Copy Markdown
Member

I still don't get how this happens, so I'd prefer to really understand what happens here before merging.

The original idea was that we would only ever grab paths once per tsconfig.json (or I guess from the code, once per compilerOptions.Paths). How would multiple of these come into play?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There were two separate parsedPatternsForPaths and parsedPatternsForPathsOnce fields before this; one on module.Resolver's caches that was implicitly dependent on the compiler options of the host program, and one on every individual resolutionState. The former was used, and shared, between any lookups inside the host program's own files. But any lookups inside project reference redirects couldn't use the resolver's cache because those redirects need the paths of the referenced project's options. So for all lookups where a redirectedReference was passed in, we just recomputed the path patterns uncached. This change consolidates the two to a single shared cache, keyed by the path mappings from potentially multiple different compiler options.

@andrewbranch
Andrew Branch (andrewbranch) added this pull request to the merge queue Aug 26, 2026
Merged via the queue into microsoft:main with commit 889659e Aug 26, 2026
25 checks passed
@github-project-automation github-project-automation Bot moved this from Needs merge to Done in PR Backlog Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Path mappings parsing uses too much memory for projects with many references and paths

5 participants