Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Always use sccache's own jobserver #2296

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ pub fn run_command(cmd: Command) -> Result<i32> {

trace!("Command::PackageToolchain({})", executable.display());
let runtime = Runtime::new()?;
let jobserver = unsafe { Client::new() };
let jobserver = Client::new();
let creator = ProcessCommandCreator::new(&jobserver);
let args: Vec<_> = env::args_os().collect();
let env: Vec<_> = env::vars_os().collect();
Expand All @@ -788,7 +788,7 @@ pub fn run_command(cmd: Command) -> Result<i32> {
env_vars,
} => {
trace!("Command::Compile {{ {:?}, {:?}, {:?} }}", exe, cmdline, cwd);
let jobserver = unsafe { Client::new() };
let jobserver = Client::new();
let conn = connect_or_start_server(&get_addr(), startup_timeout)?;
let mut runtime = Runtime::new()?;
let res = do_compile(
Expand Down
70 changes: 64 additions & 6 deletions src/jobserver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,68 @@ use futures::StreamExt;

use crate::errors::*;

// The execution model of sccache is that on the first run it spawns a server
// in the background and detaches it.
// When normally executing the rust compiler from either cargo or make, it
// will use cargo/make's jobserver and limit its resource usage accordingly.
// When executing the rust compiler through the sccache server, that jobserver
// is not available, and spawning as many rustc as there are CPUs can lead to
// a quadratic use of the CPU resources (each rustc spawning as many threads
// as there are CPUs).
// One way around this issue is to inherit the jobserver from cargo or make
// when the sccache server is spawned, but that means that in some cases, the
// cargo or make process can't terminate until the sccache server terminates
// after its idle timeout (which also never happens if SCCACHE_IDLE_TIMEOUT=0).
// Also, if the sccache server ends up shared between multiple runs of
// cargo/make, then which jobserver is used doesn't make sense anymore.
// Ideally, the sccache client would give a handle to the jobserver it has
// access to, so that the rust compiler would "just" use the jobserver it
// would have used if it had run without sccache, but that adds some extra
// complexity, and requires to use Unix domain sockets.
// What we do instead is to arbitrary use our own jobserver.
// Unfortunately, that doesn't absolve us from having to deal with the original
// jobserver, because make may give us file descriptors to its pipes, and the
// simple fact of keeping them open can block it.
// So if it does give us those file descriptors, close the preemptively.
//
// unsafe because it can use the wrong fds.
#[cfg(not(windows))]
pub unsafe fn discard_inherited_jobserver() {
if let Some(value) = ["CARGO_MAKEFLAGS", "MAKEFLAGS", "MFLAGS"]
.into_iter()
.find_map(|env| std::env::var(env).ok())
{
if let Some(auth) = value.rsplit(' ').find_map(|arg| {
arg.strip_prefix("--jobserver-auth=")
.or_else(|| arg.strip_prefix("--jobserver-fds="))
}) {
if !auth.starts_with("fifo:") {
let mut parts = auth.splitn(2, ',');
let read = parts.next().unwrap();
let write = match parts.next() {
Some(w) => w,
None => return,
};
let read = read.parse().unwrap();
let write = write.parse().unwrap();
if read < 0 || write < 0 {
return;
}
unsafe {
if libc::fcntl(read, libc::F_GETFD) == -1 {
return;
}
if libc::fcntl(write, libc::F_GETFD) == -1 {
return;
}
libc::close(read);
libc::close(write);
}
}
}
}
}

#[derive(Clone)]
pub struct Client {
helper: Option<Arc<jobserver::HelperThread>>,
Expand All @@ -20,12 +82,8 @@ pub struct Acquired {
}

impl Client {
// unsafe because `from_env` is unsafe (can use the wrong fds)
pub unsafe fn new() -> Client {
match jobserver::Client::from_env() {
Some(c) => Client::_new(c, true),
None => Client::new_num(num_cpus::get()),
}
pub fn new() -> Client {
Client::new_num(num_cpus::get())
}

pub fn new_num(num: usize) -> Client {
Expand Down
2 changes: 1 addition & 1 deletion src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ pub fn start_server(config: &Config, addr: &crate::net::SocketAddr) -> Result<()
});
panic_hook(info)
}));
let client = unsafe { Client::new() };
let client = Client::new();
let runtime = tokio::runtime::Builder::new_multi_thread()
.enable_all()
.worker_threads(std::cmp::max(20, 2 * num_cpus::get()))
Expand Down
2 changes: 1 addition & 1 deletion src/test/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ where
CacheMode::ReadWrite,
));

let client = unsafe { Client::new() };
let client = Client::new();
let srv = SccacheServer::new(0, runtime, client, dist_client, storage).unwrap();
let mut srv: SccacheServer<_, Arc<Mutex<MockCommandCreator>>> = srv;
let addr = srv.local_addr().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/test/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ macro_rules! assert_map_contains {
}

pub fn new_creator() -> Arc<Mutex<MockCommandCreator>> {
let client = unsafe { Client::new() };
let client = Client::new();
Arc::new(Mutex::new(MockCommandCreator::new(&client)))
}

Expand Down
5 changes: 5 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,7 @@ impl<'a> Hasher for HashToDigest<'a> {
/// Pipe `cmd`'s stdio to `/dev/null`, unless a specific env var is set.
#[cfg(not(windows))]
pub fn daemonize() -> Result<()> {
use crate::jobserver::discard_inherited_jobserver;
use daemonize::Daemonize;
use std::env;
use std::mem;
Expand All @@ -853,6 +854,10 @@ pub fn daemonize() -> Result<()> {
}
}

unsafe {
discard_inherited_jobserver();
}

static mut PREV_SIGSEGV: *mut libc::sigaction = 0 as *mut _;
static mut PREV_SIGBUS: *mut libc::sigaction = 0 as *mut _;
static mut PREV_SIGILL: *mut libc::sigaction = 0 as *mut _;
Expand Down
Loading