Limit client thread to 1 for Command::Compile

[zsrkmyn]

This tries to fix #2473.

[zsrkmyn]

Ooops. I just found that rustc supports parallel compilation natively. As I'm a C/C++ user, I'm not sure if the fix is correct for rust compilation.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.13%. Comparing base (764daee) to head (4467967).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2474      +/-   ##
==========================================
- Coverage   71.14%   71.13%   -0.01%     
==========================================
  Files          64       64              
  Lines       35207    35207              
==========================================
- Hits        25047    25046       -1     
- Misses      10160    10161       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ahartmetz]

Ooops. I just found that rustc supports parallel compilation natively. As I'm a C/C++ user, I'm not sure if the fix is correct for rust compilation.

I don't see how this change would make things worse. This is a change in the async runtime, which I don't think in any way propagates to the Rust compiler, a presumably exec'd child process. The only thing that might be different for Rust is that you want a lower amount of compile jobs so that the number of compiler threads doesn't greatly exceed the number of cores (that isn't currently done AFAIK). Apart from that, for Rust compilation in sccache, the question seems to be the same as for C++: what is the ideal thread count to avoid slowing down in sccache while also not wasting CPU time and memory on unused thread construction, stacks etc? Given that sccache doesn't use a lot of CPU itself, the answer might well be one, or maybe two or so.

[zsrkmyn]

@ahartmetz Thanks for the reply! Then I believe the fix is correct. IIUC, the newly created threads are used to run the compilation command only (see line 812), and sccache itself runs on the main thread; so creating 1 new thread for is enough, right?

[ahartmetz]

@ahartmetz Thanks for the reply! Then I believe the fix is correct. IIUC, the newly created threads are used to run the compilation command only (see line 812), and sccache itself runs on the main thread; so creating 1 new thread for is enough, right?

I'm only reasonably sure about the (non-)difference between C/C++ and Rust regarding this patch. I'll leave commenting about more general issues to others who know the codebase (and Rust and Rust async...) better than me.

[ahartmetz]

By the way, I think you might misunderstand something: The compiler is always run in a subprocess, which can spawn as many threads as it wants, but will nevertheless only have one communication channel to its parent process sccache, so one thread or less, if I/O multiplexing is used (which I think it might be via async), required to deal with it.

A separate issue is that one might want to spawn fewer multi-threaded compilers to avoid oversubscribing cores too much, but AFAIK even the Rust compiler often uses only a single thread, so this is a difficult trade-off to make, and just running as many compiler processes as there are hardware threads (as it's currently done) seems like a reasonable choice.