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.

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

More thread-safety changes #49882

Merged
merged 6 commits into from
Apr 17, 2018
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
3 changes: 3 additions & 0 deletions src/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/libarena/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ version = "0.0.0"
name = "arena"
path = "lib.rs"
crate-type = ["dylib"]

[dependencies]
rustc_data_structures = { path = "../librustc_data_structures" }
71 changes: 71 additions & 0 deletions src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
#![allow(deprecated)]

extern crate alloc;
extern crate rustc_data_structures;

use rustc_data_structures::sync::MTLock;

use std::cell::{Cell, RefCell};
use std::cmp;
Expand Down Expand Up @@ -290,6 +293,8 @@ pub struct DroplessArena {
chunks: RefCell<Vec<TypedArenaChunk<u8>>>,
}

unsafe impl Send for DroplessArena {}

impl DroplessArena {
pub fn new() -> DroplessArena {
DroplessArena {
Expand Down Expand Up @@ -410,6 +415,72 @@ impl DroplessArena {
}
}

pub struct SyncTypedArena<T> {
lock: MTLock<TypedArena<T>>,
}

impl<T> SyncTypedArena<T> {
#[inline(always)]
pub fn new() -> SyncTypedArena<T> {
SyncTypedArena {
lock: MTLock::new(TypedArena::new())
}
}

#[inline(always)]
pub fn alloc(&self, object: T) -> &mut T {
// Extend the lifetime of the result since it's limited to the lock guard
unsafe { &mut *(self.lock.lock().alloc(object) as *mut T) }
}

#[inline(always)]
pub fn alloc_slice(&self, slice: &[T]) -> &mut [T]
where
T: Copy,
{
// Extend the lifetime of the result since it's limited to the lock guard
unsafe { &mut *(self.lock.lock().alloc_slice(slice) as *mut [T]) }
}

#[inline(always)]
pub fn clear(&mut self) {
self.lock.get_mut().clear();
}
}

pub struct SyncDroplessArena {
lock: MTLock<DroplessArena>,
}

impl SyncDroplessArena {
#[inline(always)]
pub fn new() -> SyncDroplessArena {
SyncDroplessArena {
lock: MTLock::new(DroplessArena::new())
}
}

#[inline(always)]
pub fn in_arena<T: ?Sized>(&self, ptr: *const T) -> bool {
self.lock.lock().in_arena(ptr)
}

#[inline(always)]
pub fn alloc<T>(&self, object: T) -> &mut T {
// Extend the lifetime of the result since it's limited to the lock guard
unsafe { &mut *(self.lock.lock().alloc(object) as *mut T) }
}

#[inline(always)]
pub fn alloc_slice<T>(&self, slice: &[T]) -> &mut [T]
where
T: Copy,
{
// Extend the lifetime of the result since it's limited to the lock guard
unsafe { &mut *(self.lock.lock().alloc_slice(slice) as *mut [T]) }
}
}

#[cfg(test)]
mod tests {
extern crate test;
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use hir::print::Nested;
use hir::svh::Svh;
use util::nodemap::{DefIdMap, FxHashMap};

use arena::TypedArena;
use arena::SyncTypedArena;
use std::io;
use ty::TyCtxt;

Expand Down Expand Up @@ -219,15 +219,15 @@ impl<'hir> MapEntry<'hir> {
pub struct Forest {
krate: Crate,
pub dep_graph: DepGraph,
inlined_bodies: TypedArena<Body>
inlined_bodies: SyncTypedArena<Body>
}

impl Forest {
pub fn new(krate: Crate, dep_graph: &DepGraph) -> Forest {
Forest {
krate,
dep_graph: dep_graph.clone(),
inlined_bodies: TypedArena::new()
inlined_bodies: SyncTypedArena::new()
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use errors::DiagnosticBuilder;
use syntax_pos::{self, Span};
use syntax_pos::symbol::InternedString;
use util::nodemap::FxHashMap;
use arena::DroplessArena;
use arena::SyncDroplessArena;

use self::combine::CombineFields;
use self::higher_ranked::HrMatchResult;
Expand Down Expand Up @@ -407,15 +407,15 @@ impl fmt::Display for FixupError {
/// F: for<'b, 'tcx> where 'gcx: 'tcx FnOnce(InferCtxt<'b, 'gcx, 'tcx>).
pub struct InferCtxtBuilder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
global_tcx: TyCtxt<'a, 'gcx, 'gcx>,
arena: DroplessArena,
arena: SyncDroplessArena,
fresh_tables: Option<RefCell<ty::TypeckTables<'tcx>>>,
}

impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
pub fn infer_ctxt(self) -> InferCtxtBuilder<'a, 'gcx, 'tcx> {
InferCtxtBuilder {
global_tcx: self,
arena: DroplessArena::new(),
arena: SyncDroplessArena::new(),
fresh_tables: None,

}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use self::TargetLint::*;

use std::slice;
use rustc_data_structures::sync::{RwLock, ReadGuard};
use lint::{EarlyLintPassObject, LateLintPassObject};
use lint::{Level, Lint, LintId, LintPass, LintBuffer};
use lint::builtin::BuiltinLintDiagnostics;
Expand All @@ -39,7 +40,6 @@ use ty::layout::{LayoutError, LayoutOf, TyLayout};
use util::nodemap::FxHashMap;

use std::default::Default as StdDefault;
use std::cell::{Ref, RefCell};
use syntax::ast;
use syntax::edition;
use syntax_pos::{MultiSpan, Span};
Expand Down Expand Up @@ -78,7 +78,7 @@ pub struct LintStore {

pub struct LintSession<'a, PassObject> {
/// Reference to the store of registered lints.
lints: Ref<'a, LintStore>,
lints: ReadGuard<'a, LintStore>,

/// Trait objects for each lint pass.
passes: Option<Vec<PassObject>>,
Expand Down Expand Up @@ -336,7 +336,7 @@ impl<'a, PassObject: LintPassObject> LintSession<'a, PassObject> {
/// Creates a new `LintSession`, by moving out the `LintStore`'s initial
/// lint levels and pass objects. These can be restored using the `restore`
/// method.
fn new(store: &'a RefCell<LintStore>) -> LintSession<'a, PassObject> {
fn new(store: &'a RwLock<LintStore>) -> LintSession<'a, PassObject> {
let mut s = store.borrow_mut();
let passes = PassObject::take_passes(&mut *s);
drop(s);
Expand All @@ -347,7 +347,7 @@ impl<'a, PassObject: LintPassObject> LintSession<'a, PassObject> {
}

/// Restores the levels back to the original lint store.
fn restore(self, store: &RefCell<LintStore>) {
fn restore(self, store: &RwLock<LintStore>) {
drop(self.lints);
let mut s = store.borrow_mut();
PassObject::restore_passes(&mut *s, self.passes);
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use util::nodemap::{FxHashSet};
use util::common::{duration_to_secs_str, ErrorReported};
use util::common::ProfileQueriesMsg;

use rustc_data_structures::sync::{Lrc, Lock, LockCell, OneThread, Once};
use rustc_data_structures::sync::{Lrc, Lock, LockCell, OneThread, Once, RwLock};

use syntax::ast::NodeId;
use errors::{self, DiagnosticBuilder, DiagnosticId};
Expand Down Expand Up @@ -83,8 +83,8 @@ pub struct Session {

// FIXME: lint_store and buffered_lints are not thread-safe,
// but are only used in a single thread
pub lint_store: OneThread<RefCell<lint::LintStore>>,
pub buffered_lints: OneThread<RefCell<Option<lint::LintBuffer>>>,
pub lint_store: RwLock<lint::LintStore>,
pub buffered_lints: Lock<Option<lint::LintBuffer>>,

/// Set of (DiagnosticId, Option<Span>, message) tuples tracking
/// (sub)diagnostics that have been set once, but should not be set again,
Expand Down Expand Up @@ -1091,8 +1091,8 @@ pub fn build_session_(
default_sysroot,
local_crate_source_file,
working_dir,
lint_store: OneThread::new(RefCell::new(lint::LintStore::new())),
buffered_lints: OneThread::new(RefCell::new(Some(lint::LintBuffer::new()))),
lint_store: RwLock::new(lint::LintStore::new()),
buffered_lints: Lock::new(Some(lint::LintBuffer::new())),
one_time_diagnostics: RefCell::new(FxHashSet()),
plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
plugin_attributes: OneThread::new(RefCell::new(Vec::new())),
Expand Down
36 changes: 18 additions & 18 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,11 @@ use rustc_data_structures::accumulate_vec::AccumulateVec;
use rustc_data_structures::stable_hasher::{HashStable, hash_stable_hashmap,
StableHasher, StableHasherResult,
StableVec};
use arena::{TypedArena, DroplessArena};
use arena::{TypedArena, SyncDroplessArena};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::sync::{Lrc, Lock};
use std::any::Any;
use std::borrow::Borrow;
use std::cell::Cell;
use std::cmp::Ordering;
use std::collections::hash_map::{self, Entry};
use std::hash::{Hash, Hasher};
Expand All @@ -83,14 +82,14 @@ use hir;

pub struct AllArenas<'tcx> {
pub global: GlobalArenas<'tcx>,
pub interner: DroplessArena,
pub interner: SyncDroplessArena,
}

impl<'tcx> AllArenas<'tcx> {
pub fn new() -> Self {
AllArenas {
global: GlobalArenas::new(),
interner: DroplessArena::new(),
interner: SyncDroplessArena::new(),
}
}
}
Expand Down Expand Up @@ -130,7 +129,7 @@ type InternedSet<'tcx, T> = Lock<FxHashSet<Interned<'tcx, T>>>;

pub struct CtxtInterners<'tcx> {
/// The arena that types, regions, etc are allocated from
arena: &'tcx DroplessArena,
arena: &'tcx SyncDroplessArena,

/// Specifically use a speedy hash algorithm for these hash sets,
/// they're accessed quite often.
Expand All @@ -147,7 +146,7 @@ pub struct CtxtInterners<'tcx> {
}

impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
fn new(arena: &'tcx DroplessArena) -> CtxtInterners<'tcx> {
fn new(arena: &'tcx SyncDroplessArena) -> CtxtInterners<'tcx> {
CtxtInterners {
arena,
type_: Default::default(),
Expand All @@ -174,10 +173,10 @@ impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
return ty;
}
let global_interner = global_interners.map(|interners| {
interners.type_.borrow_mut()
(interners.type_.borrow_mut(), &interners.arena)
});
if let Some(ref interner) = global_interner {
if let Some(&Interned(ty)) = interner.get(&st) {
if let Some((ref type_, _)) = global_interner {
if let Some(&Interned(ty)) = type_.get(&st) {
return ty;
}
}
Expand All @@ -193,18 +192,18 @@ impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
// determine that all contents are in the global tcx.
// See comments on Lift for why we can't use that.
if !flags.flags.intersects(ty::TypeFlags::KEEP_IN_LOCAL_TCX) {
if let Some(interner) = global_interners {
if let Some((mut type_, arena)) = global_interner {
let ty_struct: TyS<'gcx> = unsafe {
mem::transmute(ty_struct)
};
let ty: Ty<'gcx> = interner.arena.alloc(ty_struct);
global_interner.unwrap().insert(Interned(ty));
let ty: Ty<'gcx> = arena.alloc(ty_struct);
type_.insert(Interned(ty));
return ty;
}
} else {
// Make sure we don't end up with inference
// types/regions in the global tcx.
if global_interners.is_none() {
if global_interner.is_none() {
drop(interner);
bug!("Attempted to intern `{:?}` which contains \
inference types/regions in the global type context",
Expand Down Expand Up @@ -915,9 +914,6 @@ pub struct GlobalCtxt<'tcx> {
/// Data layout specification for the current target.
pub data_layout: TargetDataLayout,

/// Used to prevent layout from recursing too deeply.
pub layout_depth: Cell<usize>,

stability_interner: Lock<FxHashSet<&'tcx attr::Stability>>,

pub interpret_interner: InterpretInterner<'tcx>,
Expand Down Expand Up @@ -1292,7 +1288,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
crate_name: Symbol::intern(crate_name),
data_layout,
layout_interner: Lock::new(FxHashSet()),
layout_depth: Cell::new(0),
stability_interner: Lock::new(FxHashSet()),
interpret_interner: Default::default(),
tx_to_llvm_workers: Lock::new(tx),
Expand Down Expand Up @@ -1559,7 +1554,7 @@ impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
/// Call the closure with a local `TyCtxt` using the given arena.
pub fn enter_local<F, R>(
&self,
arena: &'tcx DroplessArena,
arena: &'tcx SyncDroplessArena,
Copy link
Member

Choose a reason for hiding this comment

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

Could the local arenas in theory be non-synchronized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the arenas could be thread-local if we find a way to deal with lifting of types.

f: F
) -> R
where
Expand All @@ -1574,6 +1569,7 @@ impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
let new_icx = ty::tls::ImplicitCtxt {
tcx,
query: icx.query.clone(),
layout_depth: icx.layout_depth,
};
ty::tls::enter_context(&new_icx, |new_icx| {
f(new_icx.tcx)
Expand Down Expand Up @@ -1768,6 +1764,9 @@ pub mod tls {
/// The current query job, if any. This is updated by start_job in
/// ty::maps::plumbing when executing a query
pub query: Option<Lrc<maps::QueryJob<'gcx>>>,

/// Used to prevent layout from recursing too deeply.
pub layout_depth: usize,
}

// A thread local value which stores a pointer to the current ImplicitCtxt
Expand Down Expand Up @@ -1853,6 +1852,7 @@ pub mod tls {
let icx = ImplicitCtxt {
tcx,
query: None,
layout_depth: 0,
};
enter_context(&icx, |_| {
f(tcx)
Expand Down
Loading