-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
internal: VFS no longer stores all source files in memory #16307
Conversation
@Veykril looks like the sync PR is unlikely to get merged today, so feel free to merge anything that doesn't touch the proc macro server parts. |
It's fine, I'm done for the day anyways. |
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.
having touched this side of things, this is a great improvement :)
Let's get it in the nightly. @bors r+ |
☀️ Test successful - checks-actions |
After this PR, if we read a file ( Furthermore, I noticed a FIXME in |
Hmm, ye we do one round trip unnecessarily there. We can fix that, though that won't really matter too much I believe. Regarding the |
If this will not have any negative impact on the project, perhaps I can try to do it; at least it can make the project more concise. |
Feel free to take a look! |
Turns out there is no need to keep the files around. We either upload them to salsa once processed, or we need to keep them around for the
DidChangeTextDocumentNotification
, but that notification is only valid for opened documents, so instead we can just keep the files around in theMemDocs
!Fixes #16301