Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Add support for using a jobserver with Rayon #56946

Merged
merged 4 commits into from
Mar 2, 2019
Merged

Add support for using a jobserver with Rayon #56946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
892fed9
Add support for using a jobserver with Rayon
Zoxc Dec 18, 2018
35a1b91
Address comments
Zoxc Jan 31, 2019
42d817f
Fix import
Zoxc Jan 31, 2019
5c78fa8
Update Cargo.lock
Zoxc Feb 27, 2019
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
30 changes: 16 additions & 14 deletions Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2358,8 +2358,8 @@ dependencies = [
"num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
"parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
"polonius-engine 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc_apfloat 0.0.0",
"rustc_data_structures 0.0.0",
"rustc_errors 0.0.0",
Expand Down Expand Up @@ -2409,8 +2409,8 @@ dependencies = [
"rustc-ap-rustc_cratesio_shim 373.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-ap-serialize 373.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"smallvec 0.6.7 (registry+https://github.com/rust-lang/crates.io-index)",
"stable_deref_trait 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
Expand Down Expand Up @@ -2508,23 +2508,23 @@ dependencies = [

[[package]]
name = "rustc-rayon"
version = "0.1.1"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"crossbeam-deque 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"either 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "rustc-rayon-core"
version = "0.1.1"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"crossbeam-deque 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.46 (registry+https://github.com/rust-lang/crates.io-index)",
"num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
Expand Down Expand Up @@ -2672,11 +2672,13 @@ dependencies = [
"cfg-if 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
"ena 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
"graphviz 0.0.0",
"jobserver 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon-core 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc_cratesio_shim 0.0.0",
"serialize 0.0.0",
"smallvec 0.6.7 (registry+https://github.com/rust-lang/crates.io-index)",
Expand All @@ -2692,7 +2694,7 @@ dependencies = [
"graphviz 0.0.0",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc 0.0.0",
"rustc-rayon 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc_allocator 0.0.0",
"rustc_borrowck 0.0.0",
"rustc_codegen_utils 0.0.0",
Expand Down Expand Up @@ -2758,7 +2760,7 @@ version = "0.0.0"
dependencies = [
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc 0.0.0",
"rustc-rayon 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc_allocator 0.0.0",
"rustc_borrowck 0.0.0",
"rustc_codegen_utils 0.0.0",
Expand Down Expand Up @@ -4190,8 +4192,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum rustc-ap-syntax_pos 373.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8e4f88a1213562373cee9de5a1d77bbf16dd706030304af041c9733492fcc952"
"checksum rustc-demangle 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "82ae957aa1b3055d8e086486723c0ccd3d7b8fa190ae8fa2e35543b6171c810e"
"checksum rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7540fc8b0c49f096ee9c961cda096467dce8084bec6bdca2fc83895fd9b28cb8"
"checksum rustc-rayon 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "8c6d5a683c6ba4ed37959097e88d71c9e8e26659a3cb5be8b389078e7ad45306"
"checksum rustc-rayon-core 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "40f06724db71e18d68b3b946fdf890ca8c921d9edccc1404fdfdb537b0d12649"
"checksum rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "8d98c51d9cbbe810c8b6693236d3412d8cd60513ff27a3e1b6af483dca0af544"
"checksum rustc-rayon-core 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "526e7b6d2707a5b9bec3927d424ad70fa3cfc68e0ac1b75e46cdbbc95adc5108"
"checksum rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)" = "dcf128d1287d2ea9d80910b5f1120d0b8eede3fbf1abe91c40d39ea7d51e6fda"
"checksum rustc_tools_util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b3c5a95edfa0c893236ae4778bb7c4752760e4c0d245e19b5eff33c5aa5eb9dc"
"checksum rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a"
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ num_cpus = "1.0"
scoped-tls = "1.0"
log = { version = "0.4", features = ["release_max_level_info", "std"] }
polonius-engine = "0.6.2"
rustc-rayon = "0.1.1"
rustc-rayon-core = "0.1.1"
rustc-rayon = "0.1.2"
rustc-rayon-core = "0.1.2"
rustc_apfloat = { path = "../librustc_apfloat" }
rustc_target = { path = "../librustc_target" }
rustc_data_structures = { path = "../librustc_data_structures" }
Expand Down
30 changes: 3 additions & 27 deletions src/librustc/session/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ use crate::util::profiling::SelfProfiler;

use rustc_target::spec::{PanicStrategy, RelroLevel, Target, TargetTriple};
use rustc_data_structures::flock;
use jobserver::Client;
use rustc_data_structures::jobserver;
use ::jobserver::Client;

use std;
use std::cell::{self, Cell, RefCell};
Expand Down Expand Up @@ -1230,32 +1231,7 @@ pub fn build_session_(
optimization_fuel,
print_fuel_crate,
print_fuel,
// Note that this is unsafe because it may misinterpret file descriptors
// on Unix as jobserver file descriptors. We hopefully execute this near
// the beginning of the process though to ensure we don't get false
// positives, or in other words we try to execute this before we open
// any file descriptors ourselves.
//
// Pick a "reasonable maximum" if we don't otherwise have
// a jobserver in our environment, capping out at 32 so we
// don't take everything down by hogging the process run queue.
// The fixed number is used to have deterministic compilation
// across machines.
//
// Also note that we stick this in a global because there could be
// multiple `Session` instances in this process, and the jobserver is
// per-process.
jobserver: unsafe {
static mut GLOBAL_JOBSERVER: *mut Client = 0 as *mut _;
static INIT: std::sync::Once = std::sync::ONCE_INIT;
INIT.call_once(|| {
let client = Client::from_env().unwrap_or_else(|| {
Client::new(32).expect("failed to create jobserver")
});
GLOBAL_JOBSERVER = Box::into_raw(Box::new(client));
});
(*GLOBAL_JOBSERVER).clone()
},
jobserver: jobserver::client(),
has_global_allocator: Once::new(),
has_panic_handler: Once::new(),
driver_lint_caps,
Expand Down
5 changes: 5 additions & 0 deletions src/librustc/ty/query/job.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{fmt, ptr};
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::sync::{Lock, LockGuard, Lrc, Weak};
use rustc_data_structures::OnDrop;
use rustc_data_structures::jobserver;
use syntax_pos::Span;

use crate::ty::tls;
Expand Down Expand Up @@ -198,7 +199,11 @@ impl<'tcx> QueryLatch<'tcx> {
// we have to be in the `wait` call. This is ensured by the deadlock handler
// getting the self.info lock.
rayon_core::mark_blocked();
jobserver::release_thread();
waiter.condvar.wait(&mut info);
// Release the lock before we potentially block in `acquire_thread`
mem::drop(info);
jobserver::acquire_thread();
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/librustc_data_structures/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ crate-type = ["dylib"]
[dependencies]
ena = "0.11"
log = "0.4"
jobserver_crate = { version = "0.1", package = "jobserver" }
lazy_static = "1"
rustc_cratesio_shim = { path = "../librustc_cratesio_shim" }
serialize = { path = "../libserialize" }
graphviz = { path = "../libgraphviz" }
cfg-if = "0.1.2"
stable_deref_trait = "1.0.0"
rayon = { version = "0.1.1", package = "rustc-rayon" }
rayon-core = { version = "0.1.1", package = "rustc-rayon-core" }
rayon = { version = "0.1.2", package = "rustc-rayon" }
rayon-core = { version = "0.1.2", package = "rustc-rayon-core" }
rustc-hash = "1.0.1"
smallvec = { version = "0.6.7", features = ["union", "may_dangle"] }

Expand Down
156 changes: 156 additions & 0 deletions src/librustc_data_structures/jobserver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
use jobserver_crate::{Client, HelperThread, Acquired};
use lazy_static::lazy_static;
use std::sync::{Condvar, Arc, Mutex};
use std::mem;

#[derive(Default)]
struct LockedProxyData {
/// The number of free thread tokens, this may include the implicit token given to the process
free: usize,

/// The number of threads waiting for a token
waiters: usize,

/// The number of tokens we requested from the server
requested: usize,

/// Stored tokens which will be dropped when we no longer need them
tokens: Vec<Acquired>,
}

impl LockedProxyData {
fn request_token(&mut self, thread: &Mutex<HelperThread>) {
self.requested += 1;
thread.lock().unwrap().request_token();
}

fn release_token(&mut self, cond_var: &Condvar) {
if self.waiters > 0 {
self.free += 1;
cond_var.notify_one();
} else {
if self.tokens.is_empty() {
// We are returning the implicit token
self.free += 1;
} else {
// Return a real token to the server
self.tokens.pop().unwrap();
}
}
}

fn take_token(&mut self, thread: &Mutex<HelperThread>) -> bool {
if self.free > 0 {
self.free -= 1;
self.waiters -= 1;

// We stole some token reqested by someone else
// Request another one
if self.requested + self.free < self.waiters {
self.request_token(thread);
}

true
} else {
false
}
}

fn new_requested_token(&mut self, token: Acquired, cond_var: &Condvar) {
self.requested -= 1;

// Does anything need this token?
if self.waiters > 0 {
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
self.free += 1;
self.tokens.push(token);
cond_var.notify_one();
} else {
// Otherwise we'll just drop it
mem::drop(token);
}
}
}

#[derive(Default)]
struct ProxyData {
lock: Mutex<LockedProxyData>,
cond_var: Condvar,
}

/// A helper type which makes managing jobserver tokens easier.
/// It also allows you to treat the implicit token given to the process
/// in the same manner as requested tokens.
struct Proxy {
thread: Mutex<HelperThread>,
data: Arc<ProxyData>,
}

lazy_static! {
// We can only call `from_env` once per process

// Note that this is unsafe because it may misinterpret file descriptors
// on Unix as jobserver file descriptors. We hopefully execute this near
// the beginning of the process though to ensure we don't get false
// positives, or in other words we try to execute this before we open
// any file descriptors ourselves.
//
// Pick a "reasonable maximum" if we don't otherwise have
// a jobserver in our environment, capping out at 32 so we
// don't take everything down by hogging the process run queue.
// The fixed number is used to have deterministic compilation
// across machines.
//
// Also note that we stick this in a global because there could be
// multiple rustc instances in this process, and the jobserver is
// per-process.
static ref GLOBAL_CLIENT: Client = unsafe {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unsafe keyword feels problematic. In particular, it is asserting that GLOBAL_CLIENT is used early in the process, but it (locally, at least) has no way to know that. Basically, the clients of this module must invoke one of the public methods "early enough", right?

I feel like pub fn client should be declared unsafe, and parts of this comment moved onto it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think any of this is related to memory safety though.

Client::from_env().unwrap_or_else(|| {
Client::new(32).expect("failed to create jobserver")
})
};

static ref GLOBAL_PROXY: Proxy = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no ordering dependency on when GLOBAL_PROXY is created vs GLOBAL_CLIENT, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLOBAL_PROXY requires a GLOBAL_CLIENT.

let data = Arc::new(ProxyData::default());

Proxy {
data: data.clone(),
thread: Mutex::new(client().into_helper_thread(move |token| {
data.lock.lock().unwrap().new_requested_token(token.unwrap(), &data.cond_var);
}).unwrap()),
}
};
}

pub fn client() -> Client {
GLOBAL_CLIENT.clone()
}

pub fn acquire_thread() {
GLOBAL_PROXY.acquire_token();
}

pub fn release_thread() {
GLOBAL_PROXY.release_token();
}

impl Proxy {
fn release_token(&self) {
self.data.lock.lock().unwrap().release_token(&self.data.cond_var);
}

fn acquire_token(&self) {
let mut data = self.data.lock.lock().unwrap();
data.waiters += 1;
if data.take_token(&self.thread) {
return;
}
// Request a token for us
data.request_token(&self.thread);
loop {
data = self.data.cond_var.wait(data).unwrap();
if data.take_token(&self.thread) {
return;
}
}
}
}
1 change: 1 addition & 0 deletions src/librustc_data_structures/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub mod fx;
pub mod graph;
pub mod indexed_vec;
pub mod interner;
pub mod jobserver;
pub mod obligation_forest;
pub mod owning_ref;
pub mod ptr_key;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_driver/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ arena = { path = "../libarena" }
graphviz = { path = "../libgraphviz" }
log = "0.4"
env_logger = { version = "0.5", default-features = false }
rustc-rayon = "0.1.1"
rustc-rayon = "0.1.2"
scoped-tls = "1.0"
rustc = { path = "../librustc" }
rustc_allocator = { path = "../librustc_allocator" }
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_driver/driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use rustc_allocator as allocator;
use rustc_borrowck as borrowck;
use rustc_codegen_utils::codegen_backend::CodegenBackend;
use rustc_data_structures::sync::{self, Lock};
#[cfg(parallel_compiler)]
use rustc_data_structures::jobserver;
use rustc_incremental;
use rustc_metadata::creader::CrateLoader;
use rustc_metadata::cstore::{self, CStore};
Expand Down Expand Up @@ -72,6 +74,8 @@ pub fn spawn_thread_pool<F: FnOnce(config::Options) -> R + sync::Send, R: sync::
let gcx_ptr = &Lock::new(0);

let config = ThreadPoolBuilder::new()
.acquire_thread_handler(jobserver::acquire_thread)
.release_thread_handler(jobserver::release_thread)
.num_threads(Session::threads_from_count(opts.debugging_opts.threads))
.deadlock_handler(|| unsafe { ty::query::handle_deadlock() })
.stack_size(::STACK_SIZE);
Expand Down