-
Notifications
You must be signed in to change notification settings - Fork 256
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
Large files test and fixes #4002
Large files test and fixes #4002
Conversation
…afny into largeFilesTestAndFixes
I was pretty upset about the size of this PR until I realized that 80,001 of the 81,099 added lines are intentionally enormous test files. :) (Regardless, it will take me some time to review this PR since it touches so many parts of the codebase) |
Co-authored-by: Robin Salkeld <[email protected]>
…/dafny into noVerifyAndCompileFields
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.
I started reviewing this PR before realizing that it also includes the changes from #3970. I'm going to pause my review until that one's merged to avoid re-reviewing changes.
This reverts commit 67718c0.
… largeFilesTestAndFixes
Problem
When many edits were made in close succession, parsing was being done for each of these edits, despite new edits having already been made thus making the parses obsolete.
Changes
OpenDocument
andUpdateDocument
to separate threads. These two methods are marked as[Serial]
which causes them to never execute in parallel. By not letting them do any work, new calls can come in which trigger ongoing work to be cancelled and queued work not be started, thereby reducing wasted effort.For example, if request A,B,C are sent, previously work would be done for A, then B would come and work, then C would come in and work. Now, A comes in and works, during which B and C come in triggering a cancellation of work on A, and allowing B and C to be combined into a BC change which only has to be processed once.
Previously, much of the work that is done for a request, such a parsing and resolution, was already done asynchronously, but it turned out there was still significant potential synchronous work done by
OpenDocument
andUpdateDocument
, such as updating the text buffer and migrating positions in the IDE state.ChangeProcessor
is created per processedTextDocumentContentChangeEvent
.[internal error] Parser exception: The operation was canceled.
from being emitted.Caveats
DocumentManager
through concurrent code, increasing the chance of bugs and reducing maintainability.ThreadPool
to schedule UpdateDocument calls. Maybe we are working a particularTask
for too long without marking it asLongRunning
, so there are no threads available in theThreadPool
, or we're triggering slow garbage collection. Neither options seem too plausible so it would be great to investigate more.TextBuffer.ApplyTextChange
is still O(N) complexity while it could be made O(1).Testing
[internal error] Parser exception: The operation was canceled.
diagnostic, while the many fast edits one would time out without the changes in this PR.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.