Share package.json cache and skip collecting failed lookup locations in auto-imports#2855
Open
andrewbranch wants to merge 2 commits intomicrosoft:mainfrom
Open
Share package.json cache and skip collecting failed lookup locations in auto-imports#2855andrewbranch wants to merge 2 commits intomicrosoft:mainfrom
andrewbranch wants to merge 2 commits intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets reducing memory allocations in the auto-import pipeline by reusing a shared package.json parse cache across resolver instances and by skipping collection of lookup-location metadata that auto-imports don’t consume. This fits into the TypeScript-Go language server’s performance work, particularly around module resolution during completion/auto-import indexing.
Changes:
- Add
module.ResolverOptions+NewResolverWithOptionsto allow sharing apackagejson.InfoCacheand disabling lookup-location collection. - Gate recording of
failedLookupLocations/affectingLocationsbehind a resolver option to avoid unnecessary allocations. - Update auto-import registry building to create one shared
package.jsoncache and pass it into all module resolvers used during extraction/indexing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/module/resolver.go | Adds resolver options, shared package.json cache support, and conditional lookup-location recording. |
| internal/ls/autoimport/util.go | Threads resolver options through the auto-import module resolver helper. |
| internal/ls/autoimport/registry.go | Creates and reuses a shared package.json cache for auto-import indexing resolvers; enables skipping lookup locations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2780 shows a lot of allocations going to package.json parsing. I can't repro myself, but based on the profile and the report that the problem started after #2502, I think this is probably it. The failed lookup locations are also not used by auto-imports, so that should be an additional small win. I tried invoking completions in a file in Signal-Desktop as a random benchmark, and this brought total allocs down from 0.78 GB to 0.75 GB. Not huge, but maybe there's a pathological case I haven't been able to repro. Worth a try!