Cancel outstanding tasks on document edit in the language server, second edition#2714
Cancel outstanding tasks on document edit in the language server, second edition#2714Kha merged 8 commits intoleanprover:masterfrom
Conversation
|
|
@Kha, the failure in Mathlib appears to genuinely be due to this PR, although I don't understand how this PR could be causing it. Is there some subtle way that a timeout could show up as a |
|
It is certainly puzzling, we don't even trigger interruption outside the language server. Though as we saw in the first PR, there is always room for subtle breakage like increased heartbeats. Just to make doubly-sure before I investigate, do we have runs of the same mathlib commit with and without the PR? |
|
Let me do side-by-side runs tomorrow. |
|
I increased the timeout on |
|
If I understand correctly, |
|
There was another error on the mathlib testing side, but I think the error is only due to extra changes in |
|
@collares, sorry your comments are too cryptic for me. What effect did increasing the timeout have? |
|
@collares, I'm not sure if this was intentional, but it seems you've left the I'm going to reset it to match |
Amazingly, the extra result allocation seems to have triggered a mathlib heartbeat timeout
|
I've just rebased this on |
|
@semorrison Sorry for the cryptic comments! Here's my attempt to explain what happened. All I did was to push the following commit (no rebase, no force push) whose parent at the time was Then two things happened:
This corresponds to the "Mathlib branch lean-pr-testing-2714 built against this PR, but testing failed." run mentioned above by the bot. |
|
@Kha, unfortunately Mathlib/Lean combined CI is really struggling with the number of breaking changes we have in the air at the moment. I think we understand what is going on here, and Mathlib is easy to patch (by increasing a timeout), so please feel free to proceed with this one, and I'll deal with the Mathlib fallout once it hits a nightly release. |
|
Do we know whether the heartbeat increase is significant? /cc @collares |
|
It goes up from 199700 heartbeats (mathlib master, v4.2.0-rc4) to 200745 heartbeats (nightly-testing + this PR). |
|
The benchmark run at leanprover-community/mathlib4#7933 seems OK. |
|
Seems insignificant, and we just got unlucky with something extremely close to the line. |
|
I used this PR for editing core without further problems, so let's put it in stage 0. |
This was already extremely close to the limit, and becomes a tiny bit over on `nightly-2023-10-26` (no significant overall change, see discussion at leanprover/lean4#2714 (comment)). Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
This was already extremely close to the limit, and becomes a tiny bit over on `nightly-2023-10-26` (no significant overall change, see discussion at leanprover/lean4#2714 (comment)). Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
#2648 with one additional fix: while I had reviewed all
check_systemcalls in C++, it turns out there was one explicit call tocheck_interruptedI had missed and @collares found (and that didn't do anything whilecheck_interruptedwas still based on the unused thread class).