-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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 function_cache
garbage collection bug
#6555
Conversation
…e for the spans in the token map to be in sync
CodSpeed Performance ReportMerging #6555 will degrade performances by 18.37%Comparing Summary
Benchmarks breakdown
|
… instead of Arc<RwLock>.
function_cache
garbage collection bug
…d update ServerState to use it.
Nice sleuthing on this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small thing to confirm, otherwise looks great
Thanks, i'm going to merge this PR and then follow up with the suggestions for the |
## Description Bumps repo to v0.63.6 waiting on #6555
Description
Fixes a bug that would crash the language server when a function node was part of the AST.
#5967 added a function cache to the
QueryEngine
. Garbage collection needed to be called on this otherwise it returns references to types that have been cleared from the other engines during garbage collection.In the future, any other nodes that we decide to cache need to have GC applied to them or we will end up crashing the language server on the first key-pressed event.
I've also removed the gc_frequency setting and now run GC on each keystroke. Without doing this, the spans stored in the TokenMap and what was returned from the compiler were falling out of sync. This ensures that everything is always up to date and the correct spans are used. closes #5260
Finally, I've added a
launch.json
that allows for attachinglldb
to a live runningforc-lsp
process. This was a pretty cruical step for tracking down this bug so would be nice to have it easily accessible for future debugging sessions.Checklist
Breaking*
orNew Feature
labels where relevant.