Thread priorities on Windows

[matklad]

So looks like we are hitting some thread priorities problems on windows. Specifically, we run cargo check process "in the background", which, in theory, should not block our main loop from processing requests.

However, it seems like cargo check manages to monopolize the CPU, preventing important sync handlers from running: #2825 (comment)

I guess the crude fix for this would be to mark cargo check process group as low priority. If this is a correct fix, maybe it should even be done in Cargo?

However, I would expect windows thread scheduler to be fair, and so I can't actually explain why blocking happens here at the first place. Ideally, we should dig into this before rushing through with the fix. I think there might be some bug in how we arrange cargo check (like, maybe we really just block on it's output somewhere?) or there indeed might be some subtle OS-level effect.

I ... don't know anything about this part of windows API.

[matklad]

We need to revert #2826 after we fix this.

[matklad]

While we are at it, I guess we can also speed up the test a bit, by making sure we cancel pending work after typing is done?

[Speedy37]

Windows user here.

On high core count/high load the scheduler is quite garbage. I've to use processlasso to keep things responsive (it automatically lower some process priority, i.e. rustc for me :)).
This problem is not related to rust-analyser to me, it also occur with any heavily threaded build process (rustc, cmake, ...).

[wycats]

Sometime soon after this PR, rust-analyzer became useless to me. I think 196b374 might have been a mistake 😉

[matklad]

I believe windows testing was introduced after the original regression, so it was unfair to block unrelated pr on windows build occasionally failing. The current work around is to disable cargo check altogether, or to revert to the version of rust-analyzer which still has cargo check implemented on the client side.

This will be the next bug I’ll look into once I have a large enough time chunk to actually setup windows dev environment :)

[matklad]

Investigating it right now.

Results so far:

• I am unable to reproduce user-visible slowdowns
• I am able to reproduce slowness in "diagnostics don't block typing" tests if I use

  unsafe {
      let process = winapi::um::processthreadsapi::GetCurrentProcess();
      let mask = 1;
      winapi::um::winbase::SetProcessAffinityMask(process, mask);
  }

  The event handling taks 800ms in this case (as opposed to 13ms without setting affinity)
• I've checked hypothesis that this is a regression introduced by moving cargo check to the server (by disabling cargo check), and the hypothesis seems wrong: I still get 800 ms

[matklad]

I've reverted way back to 442ab3a, and still see the same pickture: the test takes 800 ms with and 25ms without affinity.

That means that reported user-visible regression in performance and the test failing win affinity are independent events. I will continue to investigate the affinity problem (as it seems like it shouldn't be happening). However, I can't investigate user-visible degradation of experience, as I don't see it myself. Would love to get a reproduction!

[kiljacken]

@matklad I can give you access to my large work project which is often quite slow with r-a, if that would be useful, with completions often taking several seconds to appear, especially when cargo check is running as well

[matklad]

@kiljacken I belive "completion is slow" and "sever perf regression" on windows are different probles. The first one is not regression, and is platform-independent.

I think users are observing big regression on windows, but I have to sift through the recent issues to make sure it is exactly the case.

[matklad]

Current depth of the rabbit hole:

With thread affinity, std::thread::Thread::unpark routinelly takes 30-40 milliseconds

[matklad]

opened https://internals.rust-lang.org/t/thread-unpark-is-extremely-slow-on-windows/11698