Use fifo-based jobserver to enable posix_spawn over fork+exec#2718
Use fifo-based jobserver to enable posix_spawn over fork+exec#2718badumbatish wants to merge 1 commit into
Conversation
jobserver performs pre_exec in its configure() stage, which pushes a closure onto the Command struct (.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/process/unix/common.rs:316). When it does this, posix_spawn doesn't execute and things fall back to .rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/process/unix/unix.rs:94, which forks and is bad for mimalloc: > Use caution when using fork in combination with either large or huge OS pages: on a fork, the OS uses copy-on-write for all pages in the original process including the huge OS pages. When any memory is now written in that area, the OS will copy the entire 1GiB huge page (or 2MiB large page) which can cause the memory usage to grow in large increments. Creating the jobserver as a named FIFO at /tmp/sccache-jobserver-<uid>/ avoids the pre_exec requirement entirely, letting we use posix_spawn. The commit falls back to pipe-based jobserver when fifo creation fails. When using musl malloc, profiling shows the overhead was at the locking stage. Swapping musl malloc for mimalloc switches the overhead to the forking stage, hence the fix. Although the fix was geared towards mimalloc, this should applies to all options as it limits the forking.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2718 +/- ##
==========================================
- Coverage 74.41% 74.36% -0.05%
==========================================
Files 70 70
Lines 39211 39411 +200
==========================================
+ Hits 29177 29309 +132
- Misses 10034 10102 +68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 4.55%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
| use std::os::unix::fs::OpenOptionsExt; | ||
|
|
||
| let dir = std::path::PathBuf::from(format!( | ||
| "/tmp/sccache-jobserver-{}", |
There was a problem hiding this comment.
| "/tmp/sccache-jobserver-{}", | |
| "{}/sccache-jobserver-{}", std::env::get_var("TMPDIR").unwrap_or("/tmp")) |
nit. See Wikipedia for more info
This commit uses a lot of unsafe, i would love some code reviews for this