Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(exthost/#3009): Call '$release*' APIs to clean-up extension host cache #3016

Merged
merged 6 commits into from
Jan 20, 2021

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Jan 19, 2021

Issue: There is a memory leak in our completion items, signature help, and codelens via the extension host - may be responsible for the SIGABRT described in #3009 (as this can occur when the JS heap is out of memory).

Defect: The extension host adds a layer on top of the language server protocol, to serve as a cache for several language features - this helps performance when resolving items. However, this caching layer relies on the client to notify it when the items are no longer in use, so they can be cleaned up. Onivim was missing this release step.

Fix: Bake in the call to release in all the relevant subscriptions. This involves picking up the cacheId which wasn't wired up in some places (like codelens), setting up the release* API, and passing it back on conclusion of the subscription.

To test this fix - I flipped the Cache.enableDebugLogging flag in the extension host and initiated several completions. Before this fix, it's obvious the caches were simply growing endlessly:

CompletionItem cache size — 1
CompletionItem cache size — 1
CompletionItem cache size — 2
CompletionItem cache size — 2
SignatureHelp cache size — 1
CompletionItem cache size — 3
CompletionItem cache size — 3
CompletionItem cache size — 4
CompletionItem cache size — 4
SignatureHelp cache size — 2
...
CompletionItem cache size — 15
CompletionItem cache size — 15
CompletionItem cache size — 16
CompletionItem cache size — 16
CompletionItem cache size — 17
CompletionItem cache size — 17
SignatureHelp cache size — 8
CompletionItem cache size — 18
CompletionItem cache size — 18
CompletionItem cache size — 19
CompletionItem cache size — 19

After this fix, though, we can observe that the cache gets cleaned:

CompletionItem cache size — 1
CompletionItem cache size — 1
CompletionItem cache size — 0
CompletionItem cache size — 0
CompletionItem cache size — 1
CompletionItem cache size — 1
CompletionItem cache size — 0
CompletionItem cache size — 0
SignatureHelp cache size — 1
SignatureHelp cache size — 0

...
CompletionItem cache size — 0
CompletionItem cache size — 0
SignatureHelp cache size — 1
SignatureHelp cache size — 0

Related #3009
Related #1058

@bryphe bryphe changed the title fix(exthost/#3009): Call '$release*' APIs to clean-up cache fix(exthost/#3009): Call '$release*' APIs to clean-up extension host cache Jan 20, 2021
@bryphe bryphe removed the WIP label Jan 20, 2021
@bryphe bryphe merged commit 2885f9f into master Jan 20, 2021
@bryphe bryphe deleted the fix/exthost/3009/release-cached-items branch January 20, 2021 18:15
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.

1 participant