Skip to content

Commit

Permalink
Fix Build::compile_objects when used in parallel
Browse files Browse the repository at this point in the history
Before this commit, each call to `Build::compile_objects` release a raw
token regardless of whether there is a token acquired by this process.

As such, if `Build::compile_objects` is used in multithreading context,
then it would call `Client::release_raw` for each thread and thus
increasing the parallelism far beyond the limit.

This commit fixed the bug by not acquiring the implicit global token
when initializing global `jobserver::Client` and only obtain token when
compiling objects.

It also replaced use of `Option<jobserver::Client>` with
`MaybeUninit<jobserver::Client>` and eliminate an `Option::unwrap()`
inside fn `jobserver`.

Signed-off-by: Jiahao XU <[email protected]>
  • Loading branch information
NobodyXu committed Jul 10, 2023
1 parent 17c8858 commit 52c03bb
Showing 1 changed file with 7 additions and 17 deletions.
24 changes: 7 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1223,11 +1223,8 @@ impl Build {

// Limit our parallelism globally with a jobserver. Start off by
// releasing our own token for this process so we can have a bit of an
// easier to write loop below. If this fails, though, then we're likely
// on Windows with the main implicit token, so we just have a bit extra
// parallelism for a bit and don't reacquire later.
// easier to write loop below.
let server = jobserver();
let reacquire = server.release_raw().is_ok();

// When compiling objects in parallel we do a few dirty tricks to speed
// things up:
Expand Down Expand Up @@ -1265,29 +1262,24 @@ impl Build {
wait_on_child(&cmd, &program, &mut child.0)?;
}

// Reacquire our process's token before we proceed, which we released
// before entering the loop above.
if reacquire {
server.acquire_raw()?;
}

return Ok(());

/// Returns a suitable `jobserver::Client` used to coordinate
/// parallelism between build scripts.
fn jobserver() -> &'static jobserver::Client {
use std::mem::MaybeUninit;

static INIT: Once = Once::new();
static mut JOBSERVER: Option<jobserver::Client> = None;
static mut JOBSERVER: MaybeUninit<jobserver::Client> = MaybeUninit::uninit();

fn _assert_sync<T: Sync>() {}
_assert_sync::<jobserver::Client>();

unsafe {
INIT.call_once(|| {
let server = default_jobserver();
JOBSERVER = Some(server);
JOBSERVER.write(default_jobserver());
});
JOBSERVER.as_ref().unwrap()
JOBSERVER.assume_init_ref()
}
}

Expand All @@ -1313,9 +1305,7 @@ impl Build {

// If we create our own jobserver then be sure to reserve one token
// for ourselves.
let client = jobserver::Client::new(parallelism).expect("failed to create jobserver");
client.acquire_raw().expect("failed to acquire initial");
return client;
jobserver::Client::new(parallelism).expect("failed to create jobserver")
}

struct KillOnDrop(Child);
Expand Down

0 comments on commit 52c03bb

Please sign in to comment.