Skip to content

Commit

Permalink
Auto merge of #72256 - ecstatic-morse:once-cell, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Use `once_cell` crate instead of custom data structure

Internally, we use the [`Once`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/sync/struct.Once.html) type for shared data that is initialized exactly once and only read from afterwards. `Once` uses a `parking_lot::Mutex` when the parallel compiler is enabled and a `RefCell` when it is not. This PR switches to the [`once_cell`](https://crates.io/crates/once_cell) crate, which also uses a `parking_lot::Mutex` for its `sync` version (because we enable the `parking_lot` feature) but has zero overhead for its `unsync` one.

This PR adds `once_cell` to the list of whitelisted dependencies. I think this is acceptable because it is already used in `rustc_driver`, is owned by a well-known community member (cc @matklad), and has a stable release. cc @rust-lang/compiler

`once_cell` has a slightly more minimal API than `Once`, which allows for initialization to be either optimistic (evaluate the initializer and then synchronize) or pessimistic (synchronize and then evaluate the initializer). `once_cell`'s `get_or_init` is always pessimistic. The optimistic version is only used once in the current `master`.

r? @Mark-Simulacrum
  • Loading branch information
bors committed May 23, 2020
2 parents 215f2d3 + 307153e commit 7f940ef
Show file tree
Hide file tree
Showing 43 changed files with 139 additions and 249 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2327,6 +2327,9 @@ name = "once_cell"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d6a04cb71e910d0034815600180f62a95bf6e67942d7ab52a166a68c7d7e9cd0"
dependencies = [
"parking_lot 0.9.0",
]

[[package]]
name = "opaque-debug"
Expand Down Expand Up @@ -3791,6 +3794,7 @@ dependencies = [
"libc",
"log",
"measureme",
"once_cell",
"parking_lot 0.10.2",
"rustc-hash",
"rustc-rayon",
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ pub fn lower_crate<'a, 'hir>(
let _prof_timer = sess.prof.verbose_generic_activity("hir_lowering");

LoweringContext {
crate_root: sess.parse_sess.injected_crate_name.try_get().copied(),
crate_root: sess.parse_sess.injected_crate_name.get().copied(),
sess,
resolver,
nt_to_tokenstream,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub unsafe fn create_module(
llvm::LLVMRustSetModulePICLevel(llmod);
// PIE is potentially more effective than PIC, but can only be used in executables.
// If all our outputs are executables, then we can relax PIC to PIE.
if sess.crate_types.get().iter().all(|ty| *ty == CrateType::Executable) {
if sess.crate_types().iter().all(|ty| *ty == CrateType::Executable) {
llvm::LLVMRustSetModulePIELevel(llmod);
}
}
Expand Down
9 changes: 3 additions & 6 deletions src/librustc_codegen_ssa/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(
) {
let _timer = sess.timer("link_binary");
let output_metadata = sess.opts.output_types.contains_key(&OutputType::Metadata);
for &crate_type in sess.crate_types.borrow().iter() {
for &crate_type in sess.crate_types().iter() {
// Ignore executable crates if we have -Z no-codegen, as they will error.
if (sess.opts.debugging_opts.no_codegen || !sess.opts.output_types.should_codegen())
&& !output_metadata
Expand Down Expand Up @@ -875,11 +875,8 @@ fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool {

// If we're only producing artifacts that are archives, no need to preserve
// the objects as they're losslessly contained inside the archives.
let output_linked = sess
.crate_types
.borrow()
.iter()
.any(|&x| x != CrateType::Rlib && x != CrateType::Staticlib);
let output_linked =
sess.crate_types().iter().any(|&x| x != CrateType::Rlib && x != CrateType::Staticlib);
if !output_linked {
return false;
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_codegen_ssa/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ impl LinkerInfo {
LinkerInfo {
exports: tcx
.sess
.crate_types
.borrow()
.crate_types()
.iter()
.map(|&c| (c, exported_symbols(tcx, c)))
.collect(),
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_middle::ty::{SymbolName, TyCtxt};
use rustc_session::config::{CrateType, Sanitizer};

pub fn threshold(tcx: TyCtxt<'_>) -> SymbolExportLevel {
crates_export_threshold(&tcx.sess.crate_types.borrow())
crates_export_threshold(&tcx.sess.crate_types())
}

fn crate_export_threshold(crate_type: CrateType) -> SymbolExportLevel {
Expand Down Expand Up @@ -212,7 +212,7 @@ fn exported_symbols_provider_local(
}));
}

if tcx.sess.crate_types.borrow().contains(&CrateType::Dylib) {
if tcx.sess.crate_types().contains(&CrateType::Dylib) {
let symbol_name = metadata_symbol_name(tcx);
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(&symbol_name));

Expand Down
6 changes: 3 additions & 3 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ pub struct CompiledModules {

fn need_bitcode_in_object(sess: &Session) -> bool {
let requested_for_rlib = sess.opts.cg.embed_bitcode
&& sess.crate_types.borrow().contains(&CrateType::Rlib)
&& sess.crate_types().contains(&CrateType::Rlib)
&& sess.opts.output_types.contains_key(&OutputType::Exe);
let forced_by_target = sess.target.target.options.forces_embed_bitcode;
requested_for_rlib || forced_by_target
Expand Down Expand Up @@ -991,7 +991,7 @@ fn start_executing_work<B: ExtraBackendMethods>(
};
let cgcx = CodegenContext::<B> {
backend: backend.clone(),
crate_types: sess.crate_types.borrow().clone(),
crate_types: sess.crate_types().to_vec(),
each_linked_rlib_for_lto,
lto: sess.lto(),
no_landing_pads: sess.panic_strategy() == PanicStrategy::Abort,
Expand Down Expand Up @@ -1812,7 +1812,7 @@ fn msvc_imps_needed(tcx: TyCtxt<'_>) -> bool {
);

tcx.sess.target.target.options.is_like_msvc &&
tcx.sess.crate_types.borrow().iter().any(|ct| *ct == CrateType::Rlib) &&
tcx.sess.crate_types().iter().any(|ct| *ct == CrateType::Rlib) &&
// ThinLTO can't handle this workaround in all cases, so we don't
// emit the `__imp_` symbols. Instead we make them unnecessary by disallowing
// dynamic linking when linker plugin LTO is enabled.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ fn determine_cgu_reuse<'tcx>(tcx: TyCtxt<'tcx>, cgu: &CodegenUnit<'tcx>) -> CguR
match compute_per_cgu_lto_type(
&tcx.sess.lto(),
&tcx.sess.opts,
&tcx.sess.crate_types.borrow(),
&tcx.sess.crate_types(),
ModuleKind::Regular,
) {
ComputedLtoType::No => CguReuse::PostLto,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_data_structures/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ indexmap = "1"
log = "0.4"
jobserver_crate = { version = "0.1.13", package = "jobserver" }
lazy_static = "1"
once_cell = { version = "1", features = ["parking_lot"] }
rustc_serialize = { path = "../libserialize", package = "serialize" }
graphviz = { path = "../libgraphviz" }
cfg-if = "0.1.2"
Expand Down
133 changes: 4 additions & 129 deletions src/librustc_data_structures/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
use crate::owning_ref::{Erased, OwningRef};
use std::collections::HashMap;
use std::hash::{BuildHasher, Hash};
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};

pub use std::sync::atomic::Ordering;
Expand Down Expand Up @@ -230,6 +229,8 @@ cfg_if! {
pub use std::cell::RefMut as LockGuard;
pub use std::cell::RefMut as MappedLockGuard;

pub use once_cell::unsync::OnceCell;

use std::cell::RefCell as InnerRwLock;
use std::cell::RefCell as InnerLock;

Expand Down Expand Up @@ -313,6 +314,8 @@ cfg_if! {
pub use parking_lot::MutexGuard as LockGuard;
pub use parking_lot::MappedMutexGuard as MappedLockGuard;

pub use once_cell::sync::OnceCell;

pub use std::sync::atomic::{AtomicBool, AtomicUsize, AtomicU32, AtomicU64};

pub use crossbeam_utils::atomic::AtomicCell;
Expand Down Expand Up @@ -432,134 +435,6 @@ impl<K: Eq + Hash, V: Eq, S: BuildHasher> HashMapExt<K, V> for HashMap<K, V, S>
}
}

/// A type whose inner value can be written once and then will stay read-only
// This contains a PhantomData<T> since this type conceptually owns a T outside the Mutex once
// initialized. This ensures that Once<T> is Sync only if T is. If we did not have PhantomData<T>
// we could send a &Once<Cell<bool>> to multiple threads and call `get` on it to get access
// to &Cell<bool> on those threads.
pub struct Once<T>(Lock<Option<T>>, PhantomData<T>);

impl<T> Once<T> {
/// Creates an Once value which is uninitialized
#[inline(always)]
pub fn new() -> Self {
Once(Lock::new(None), PhantomData)
}

/// Consumes the value and returns Some(T) if it was initialized
#[inline(always)]
pub fn into_inner(self) -> Option<T> {
self.0.into_inner()
}

/// Tries to initialize the inner value to `value`.
/// Returns `None` if the inner value was uninitialized and `value` was consumed setting it
/// otherwise if the inner value was already set it returns `value` back to the caller
#[inline]
pub fn try_set(&self, value: T) -> Option<T> {
let mut lock = self.0.lock();
if lock.is_some() {
return Some(value);
}
*lock = Some(value);
None
}

/// Tries to initialize the inner value to `value`.
/// Returns `None` if the inner value was uninitialized and `value` was consumed setting it
/// otherwise if the inner value was already set it asserts that `value` is equal to the inner
/// value and then returns `value` back to the caller
#[inline]
pub fn try_set_same(&self, value: T) -> Option<T>
where
T: Eq,
{
let mut lock = self.0.lock();
if let Some(ref inner) = *lock {
assert!(*inner == value);
return Some(value);
}
*lock = Some(value);
None
}

/// Tries to initialize the inner value to `value` and panics if it was already initialized
#[inline]
pub fn set(&self, value: T) {
assert!(self.try_set(value).is_none());
}

/// Initializes the inner value if it wasn't already done by calling the provided closure. It
/// ensures that no-one else can access the value in the mean time by holding a lock for the
/// duration of the closure.
/// A reference to the inner value is returned.
#[inline]
pub fn init_locking<F: FnOnce() -> T>(&self, f: F) -> &T {
{
let mut lock = self.0.lock();
if lock.is_none() {
*lock = Some(f());
}
}

self.borrow()
}

/// Tries to initialize the inner value by calling the closure without ensuring that no-one
/// else can access it. This mean when this is called from multiple threads, multiple
/// closures may concurrently be computing a value which the inner value should take.
/// Only one of these closures are used to actually initialize the value.
/// If some other closure already set the value,
/// we return the value our closure computed wrapped in a `Option`.
/// If our closure set the value, `None` is returned.
/// If the value is already initialized, the closure is not called and `None` is returned.
#[inline]
pub fn init_nonlocking<F: FnOnce() -> T>(&self, f: F) -> Option<T> {
if self.0.lock().is_some() { None } else { self.try_set(f()) }
}

/// Tries to initialize the inner value by calling the closure without ensuring that no-one
/// else can access it. This mean when this is called from multiple threads, multiple
/// closures may concurrently be computing a value which the inner value should take.
/// Only one of these closures are used to actually initialize the value.
/// If some other closure already set the value, we assert that it our closure computed
/// a value equal to the value already set and then
/// we return the value our closure computed wrapped in a `Option`.
/// If our closure set the value, `None` is returned.
/// If the value is already initialized, the closure is not called and `None` is returned.
#[inline]
pub fn init_nonlocking_same<F: FnOnce() -> T>(&self, f: F) -> Option<T>
where
T: Eq,
{
if self.0.lock().is_some() { None } else { self.try_set_same(f()) }
}

/// Tries to get a reference to the inner value, returns `None` if it is not yet initialized
#[inline(always)]
pub fn try_get(&self) -> Option<&T> {
let lock = &*self.0.lock();
if let Some(ref inner) = *lock {
// This is safe since we won't mutate the inner value
unsafe { Some(&*(inner as *const T)) }
} else {
None
}
}

/// Gets reference to the inner value, panics if it is not yet initialized
#[inline(always)]
pub fn get(&self) -> &T {
self.try_get().expect("value was not set")
}

/// Gets reference to the inner value, panics if it is not yet initialized
#[inline(always)]
pub fn borrow(&self) -> &T {
self.get()
}
}

#[derive(Debug)]
pub struct Lock<T>(InnerLock<T>);

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ impl RustcDefaultCalls {
if let Input::File(file) = compiler.input() {
// FIXME: #![crate_type] and #![crate_name] support not implemented yet
let attrs = vec![];
sess.crate_types.set(collect_crate_types(sess, &attrs));
sess.init_crate_types(collect_crate_types(sess, &attrs));
let outputs = compiler.build_output_filenames(&sess, &attrs);
let rlink_data = fs::read_to_string(file).unwrap_or_else(|err| {
sess.fatal(&format!("failed to read rlink file: {}", err));
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_driver/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ pub fn print_after_parsing(
annotation.pp_ann(),
false,
parse.edition,
parse.injected_crate_name.try_get().is_some(),
parse.injected_crate_name.get().is_some(),
)
})
} else {
Expand Down Expand Up @@ -438,7 +438,7 @@ pub fn print_after_hir_lowering<'tcx>(
annotation.pp_ann(),
true,
parse.edition,
parse.injected_crate_name.try_get().is_some(),
parse.injected_crate_name.get().is_some(),
)
})
}
Expand Down
Loading

0 comments on commit 7f940ef

Please sign in to comment.