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

Cancel outstanding tasks on document edit in the language server #2648

Merged
merged 7 commits into from
Oct 13, 2023

Conversation

Kha
Copy link
Member

@Kha Kha commented Oct 10, 2023

Dramatically reduces system load and increases responsiveness of the language server. In my tests, I was not able to get the server above 100% CPU anymore.

The check_system calls inside pure Lean functions are a bit problematic. I did not find any process terminations from cancellation anymore after blacklisting two of them in the last commit but I may have missed some of them.

Comment on lines -149 to -161
inductive KernelException
0 | unknownConstant (env : Environment) (name : Name)
1 | alreadyDeclared (env : Environment) (name : Name)
2 | declTypeMismatch (env : Environment) (decl : Declaration) (givenType : Expr)
3 | declHasMVars (env : Environment) (name : Name) (expr : Expr)
4 | declHasFVars (env : Environment) (name : Name) (expr : Expr)
5 | funExpected (env : Environment) (lctx : LocalContext) (expr : Expr)
6 | typeExpected (env : Environment) (lctx : LocalContext) (expr : Expr)
7 | letTypeMismatch (env : Environment) (lctx : LocalContext) (name : Name) (givenType : Expr) (expectedType : Expr)
8 | exprTypeMismatch (env : Environment) (lctx : LocalContext) (expr : Expr) (expectedType : Expr)
9 | appTypeMismatch (env : Environment) (lctx : LocalContext) (app : Expr) (funType : Expr) (argType : Expr)
10 | invalidProj (env : Environment) (lctx : LocalContext) (proj : Expr)
11 | other (msg : String)
Copy link
Member Author

Choose a reason for hiding this comment

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

This listing was outdated and is replicated below

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Oct 12, 2023
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 12, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Oct 12, 2023
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Oct 12, 2023

kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 12, 2023
Amazingly, the extra result allocation seems to have triggered a mathlib
heartbeat timeout
@Kha
Copy link
Member Author

Kha commented Oct 12, 2023

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 61b2bfa.
There were no significant changes against commit ca0e6b0.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 12, 2023
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 12, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Oct 12, 2023
@Kha Kha added the changelog Automatically adds PR title linking to PR to changelog on merge label Oct 13, 2023
@Kha Kha merged commit 6494af4 into leanprover:master Oct 13, 2023
17 checks passed
@Kha Kha deleted the cancel branch October 13, 2023 07:52
@collares
Copy link
Contributor

collares commented Oct 17, 2023

@Kha This was reverted due to crashes, and I have a worker stack trace at https://gist.github.com/collares/ae3a8d5b5f7c6976b8d46b5bfb6e945b (commit d15a0a4, Debug). It seems like the kernel calls check_interrupted() directly in for_each_fn::apply:

check_interrupted();
check_memory("expression traversal");

@gebner
Copy link
Member

gebner commented Oct 17, 2023

FWIW, #2036 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR changelog Automatically adds PR title linking to PR to changelog on merge toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants