fix: use single-threaded tokio runtime in sccache client #2460
Conversation
|
could you please add a test to make sure we don't regress in the future on this? thanks |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2460 +/- ##
==========================================
- Coverage 71.17% 71.17% -0.01%
==========================================
Files 64 64
Lines 35592 35602 +10
==========================================
+ Hits 25334 25341 +7
- Misses 10258 10261 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sylvestre
force-pushed
the
venkyt/single-threaded-client
branch
from
e1aea58 to
68adae5
Compare
Will do. Doing a few more tests manually right now. Will add a test before taking this PR out of draft state. |
|
ping ? |
|
Sorry, got caught up with a bunch of things. This is still on my radar. Hope to get to this in the next couple of weeks. |
|
See also: #2474 |
The sccache client was using Runtime::new() which creates a multi-threaded tokio runtime with worker threads equal to the number of CPU cores. On high-core-count servers running many parallel builds, this created an excessive number of threads. For example: - 96 vCPU server - 10 concurrent make invocations - Each make using -j16 - Result: 96 × 10 × 16 = 15,360 threads created by sccache wrappers While these threads are short-lived, with continuous build pipelines this constant thread creation/destruction causes unnecessary overhead. The sccache client only performs simple I/O operations (connecting to server, sending requests, receiving responses) and doesn't need worker threads. This change replaces all client-side Runtime::new() calls with Builder::new_current_thread().enable_all().build() to use a single-threaded runtime, reducing thread count from num_cpus to 1 per client invocation.
sylvestre
force-pushed
the
venkyt/single-threaded-client
branch
from
f9ebf22 to
5a3519a
Compare
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
|
Oh, I didn't notice this PR before. It's so interesting that the issue exists for years but we both sent the PR within the same month LOL. I'm fine to drop my PR if this is a more complete fix for #2473 . |
| let tempdir = tempfile::Builder::new().prefix("sccache").tempdir()?; | ||
| let socket_path = tempdir.path().join("sock"); | ||
| let runtime = Runtime::new()?; | ||
| let runtime = tokio::runtime::Builder::new_current_thread() |
There was a problem hiding this comment.
nit: should we allow running the server on more than one core? Only one server process should be used for all client connections.
P.S. Maybe the server concurrency should be limited to 4 or 8 threads and not executed on the all cores. And we should be ready for less than 4-core systems too (just use min of constant and hardware concurrency)
6 participants
The sccache client was using Runtime::new() which creates a multi-threaded tokio runtime with worker threads equal to the number of CPU cores. On high-core-count servers running many parallel builds, this created an excessive number of threads.
For example:
While these threads are short-lived, with continuous build pipelines this constant thread creation/destruction causes unnecessary overhead. The sccache client only performs simple I/O operations (connecting to server, sending requests, receiving responses) and doesn't need worker threads.
This change replaces all client-side Runtime::new() calls with Builder::new_current_thread().enable_all().build() to use a single-threaded runtime, reducing thread count from num_cpus to 1 per client invocation.