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

Fixed verification ordering priority and speedup #2352

Merged
merged 25 commits into from
Jul 8, 2022

Conversation

MikaelMayer
Copy link
Member

@MikaelMayer MikaelMayer commented Jul 1, 2022

Fixes #2347
Also fixes the number of cores to the engine option which was never set, so it defaulted to 1.

Before this PR, on an 8-core machine with vscore set to 8 in VSCode:
0e7e68fa-5dee-42d5-9b41-4762ad620995

After this PR, same machine, same options
19293997-9794-4b1c-b0a5-08c7dba44322
Note the small grey bar where it should be green. This is a video encoding issue.

Can you spot the 4 differences?

Area Before this PR With this PR
Status bar It shows that it's verifying ALL the methods It only shows at most 8 methods being actively verified
Gutter verification finished Only one method is actually verified at a time 8 are verified at the same time
Gutter verification verifying All methods have an indicator that they are "verifying" Only the methods actively being verified have the animated icons, others have the still icon
Verification order Methods are verified from top to bottom, no matter what Method last edited are modified first

I did not write tests for verifying ordering as I found such tests used to be unstable and randomly crash the CI. I only did unit testing to verify that the priority is correctly assigned, but that PR fixes the fact that priority setting was done after verification was launched, which was useless.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@MikaelMayer MikaelMayer added area: performance Performance issues part: language server Support for LSP in Dafny (server part; client is in ide-vscode repo) kind: language development speed Slows down development of Dafny the language, flaky tests release-blocker Must be resolved before the next release labels Jul 1, 2022
@keyboardDrummer
Copy link
Member

Niiice!

I did make a few changes, hope that's OK. I've changed the verification ordering tests so they test against the API instead of internals, and I made two refactorings.

keyboardDrummer
keyboardDrummer previously approved these changes Jul 4, 2022
keyboardDrummer
keyboardDrummer previously approved these changes Jul 5, 2022
@MikaelMayer MikaelMayer enabled auto-merge (squash) July 5, 2022 15:41
});
}

private Position GetPositionOf(string code, string symbol) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny would be if we could use LSP's workspace symbol feature to ask for the positions ^^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish too.

keyboardDrummer
keyboardDrummer previously approved these changes Jul 5, 2022
@MikaelMayer MikaelMayer merged commit ff8a91f into master Jul 8, 2022
@MikaelMayer MikaelMayer deleted the fix-verification-ordering-priority branch July 8, 2022 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: performance Performance issues kind: language development speed Slows down development of Dafny the language, flaky tests part: language server Support for LSP in Dafny (server part; client is in ide-vscode repo) release-blocker Must be resolved before the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic priority on verification never worked + misleading "verifying" gutter icons
2 participants