Skip to content

Avoid a few locks #109117

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

Merged
merged 9 commits into from
Apr 5, 2023
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 Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1418,9 +1418,9 @@ dependencies = [

[[package]]
name = "elsa"
version = "1.8.0"
version = "1.7.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f74077c3c3aedb99a2683919698285596662518ea13e5eedcf8bdd43b0d0453b"
checksum = "848fe615fbb0a74d9ae68dcaa510106d32e37d9416207bbea4bd008bd89c47ed"
dependencies = [
"stable_deref_trait",
]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ stacker = "0.1.15"
tempfile = "3.2"
thin-vec = "0.2.12"
tracing = "0.1"
elsa = "1.8"
elsa = "=1.7.1"

[dependencies.parking_lot]
version = "0.11"
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_data_structures/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};
pub use std::sync::atomic::Ordering;
pub use std::sync::atomic::Ordering::SeqCst;

pub use vec::AppendOnlyVec;
pub use vec::{AppendOnlyIndexVec, AppendOnlyVec};

mod vec;

Expand Down Expand Up @@ -107,6 +107,14 @@ cfg_if! {
}
}

impl Atomic<bool> {
pub fn fetch_or(&self, val: bool, _: Ordering) -> bool {
let result = self.0.get() | val;
self.0.set(val);
result
}
}

impl<T: Copy + PartialEq> Atomic<T> {
#[inline]
pub fn compare_exchange(&self,
Expand Down Expand Up @@ -481,14 +489,6 @@ impl<T: Default> Default for Lock<T> {
}
}

// FIXME: Probably a bad idea
impl<T: Clone> Clone for Lock<T> {
#[inline]
fn clone(&self) -> Self {
Lock::new(self.borrow().clone())
}
}

#[derive(Debug, Default)]
pub struct RwLock<T>(InnerRwLock<T>);

Expand Down
68 changes: 66 additions & 2 deletions compiler/rustc_data_structures/src/sync/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ use std::marker::PhantomData;

use rustc_index::vec::Idx;

pub struct AppendOnlyVec<I: Idx, T: Copy> {
#[derive(Default)]
pub struct AppendOnlyIndexVec<I: Idx, T: Copy> {
#[cfg(not(parallel_compiler))]
vec: elsa::vec::FrozenVec<T>,
#[cfg(parallel_compiler)]
vec: elsa::sync::LockFreeFrozenVec<T>,
_marker: PhantomData<fn(&I)>,
}

impl<I: Idx, T: Copy> AppendOnlyVec<I, T> {
impl<I: Idx, T: Copy> AppendOnlyIndexVec<I, T> {
pub fn new() -> Self {
Self {
#[cfg(not(parallel_compiler))]
Expand Down Expand Up @@ -39,3 +40,66 @@ impl<I: Idx, T: Copy> AppendOnlyVec<I, T> {
return self.vec.get(i);
}
}

#[derive(Default)]
pub struct AppendOnlyVec<T: Copy> {
#[cfg(not(parallel_compiler))]
vec: elsa::vec::FrozenVec<T>,
#[cfg(parallel_compiler)]
vec: elsa::sync::LockFreeFrozenVec<T>,
}

impl<T: Copy> AppendOnlyVec<T> {
pub fn new() -> Self {
Self {
#[cfg(not(parallel_compiler))]
vec: elsa::vec::FrozenVec::new(),
#[cfg(parallel_compiler)]
vec: elsa::sync::LockFreeFrozenVec::new(),
}
}

pub fn push(&self, val: T) -> usize {
#[cfg(not(parallel_compiler))]
let i = self.vec.len();
#[cfg(not(parallel_compiler))]
self.vec.push(val);
#[cfg(parallel_compiler)]
let i = self.vec.push(val);
i
}

pub fn get(&self, i: usize) -> Option<T> {
#[cfg(not(parallel_compiler))]
return self.vec.get_copy(i);
#[cfg(parallel_compiler)]
return self.vec.get(i);
}

pub fn iter_enumerated(&self) -> impl Iterator<Item = (usize, T)> + '_ {
(0..)
.map(|i| (i, self.get(i)))
.take_while(|(_, o)| o.is_some())
.filter_map(|(i, o)| Some((i, o?)))
}

pub fn iter(&self) -> impl Iterator<Item = T> + '_ {
(0..).map(|i| self.get(i)).take_while(|o| o.is_some()).filter_map(|o| o)
}
}

impl<T: Copy + PartialEq> AppendOnlyVec<T> {
pub fn contains(&self, val: T) -> bool {
self.iter_enumerated().any(|(_, v)| v == val)
}
}

impl<A: Copy> FromIterator<A> for AppendOnlyVec<A> {
fn from_iter<T: IntoIterator<Item = A>>(iter: T) -> Self {
let this = Self::new();
for val in iter {
this.push(val);
}
this
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_codegen_ssa::traits::CodegenBackend;
use rustc_codegen_ssa::CodegenResults;
use rustc_data_structures::steal::Steal;
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{AppendOnlyVec, Lrc, OnceCell, RwLock, WorkerLocal};
use rustc_data_structures::sync::{AppendOnlyIndexVec, Lrc, OnceCell, RwLock, WorkerLocal};
use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::definitions::Definitions;
use rustc_incremental::DepGraphFuture;
Expand Down Expand Up @@ -215,7 +215,7 @@ impl<'tcx> Queries<'tcx> {

let cstore = RwLock::new(Box::new(CStore::new(sess)) as _);
let definitions = RwLock::new(Definitions::new(sess.local_stable_crate_id()));
let source_span = AppendOnlyVec::new();
let source_span = AppendOnlyIndexVec::new();
let _id = source_span.push(krate.spans.inner_span);
debug_assert_eq!(_id, CRATE_DEF_ID);
let untracked = Untracked { cstore, source_span, definitions };
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1947,7 +1947,7 @@ impl KeywordIdents {
};

// Don't lint `r#foo`.
if cx.sess().parse_sess.raw_identifier_spans.borrow().contains(&ident.span) {
if cx.sess().parse_sess.raw_identifier_spans.contains(ident.span) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl CStore {
fn push_dependencies_in_postorder(&self, deps: &mut Vec<CrateNum>, cnum: CrateNum) {
if !deps.contains(&cnum) {
let data = self.get_crate_data(cnum);
for &dep in data.dependencies().iter() {
for dep in data.dependencies() {
if dep != cnum {
self.push_dependencies_in_postorder(deps, dep);
}
Expand Down Expand Up @@ -605,7 +605,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
if cmeta.update_extern_crate(extern_crate) {
// Propagate the extern crate info to dependencies if it was updated.
let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate };
for &dep_cnum in cmeta.dependencies().iter() {
for dep_cnum in cmeta.dependencies() {
self.update_extern_crate(dep_cnum, extern_crate);
}
}
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_ast as ast;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{Lock, LockGuard, Lrc, OnceCell};
use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc, OnceCell};
use rustc_data_structures::unhash::UnhashMap;
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
Expand Down Expand Up @@ -109,7 +109,7 @@ pub(crate) struct CrateMetadata {
/// IDs as they are seen from the current compilation session.
cnum_map: CrateNumMap,
/// Same ID set as `cnum_map` plus maybe some injected crates like panic runtime.
dependencies: Lock<Vec<CrateNum>>,
dependencies: AppendOnlyVec<CrateNum>,
/// How to link (or not link) this crate to the currently compiled crate.
dep_kind: Lock<CrateDepKind>,
/// Filesystem location of this crate.
Expand Down Expand Up @@ -1594,7 +1594,7 @@ impl CrateMetadata {
.collect();
let alloc_decoding_state =
AllocDecodingState::new(root.interpret_alloc_index.decode(&blob).collect());
let dependencies = Lock::new(cnum_map.iter().cloned().collect());
let dependencies = cnum_map.iter().copied().collect();

// Pre-decode the DefPathHash->DefIndex table. This is a cheap operation
// that does not copy any data. It just does some data verification.
Expand Down Expand Up @@ -1634,12 +1634,12 @@ impl CrateMetadata {
cdata
}

pub(crate) fn dependencies(&self) -> LockGuard<'_, Vec<CrateNum>> {
self.dependencies.borrow()
pub(crate) fn dependencies(&self) -> impl Iterator<Item = CrateNum> + '_ {
self.dependencies.iter()
}

pub(crate) fn add_dependency(&self, cnum: CrateNum) {
self.dependencies.borrow_mut().push(cnum);
self.dependencies.push(cnum);
}

pub(crate) fn update_extern_crate(&self, new_extern_crate: ExternCrate) -> bool {
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1712,8 +1712,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let stability = tcx.lookup_stability(CRATE_DEF_ID);
let macros =
self.lazy_array(tcx.resolutions(()).proc_macros.iter().map(|p| p.local_def_index));
let spans = self.tcx.sess.parse_sess.proc_macro_quoted_spans();
for (i, span) in spans.into_iter().enumerate() {
for (i, span) in self.tcx.sess.parse_sess.proc_macro_quoted_spans() {
let span = self.lazy(span);
self.tables.proc_macro_quoted_spans.set_some(i, span);
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ impl AllocDecodingState {
}

pub fn new(data_offsets: Vec<u32>) -> Self {
let decoding_state = vec![Lock::new(State::Empty); data_offsets.len()];
let decoding_state =
std::iter::repeat_with(|| Lock::new(State::Empty)).take(data_offsets.len()).collect();

Self { decoding_state, data_offsets }
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<'a> StringReader<'a> {
if !sym.can_be_raw() {
self.sess.emit_err(errors::CannotBeRawIdent { span, ident: sym });
}
self.sess.raw_identifier_spans.borrow_mut().push(span);
self.sess.raw_identifier_spans.push(span);
token::Ident(sym, true)
}
rustc_lexer::TokenKind::UnknownPrefix => {
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use rustc_ast::{Async, AttrArgs, AttrArgsEq, Expr, ExprKind, MacDelimiter, Mutab
use rustc_ast::{HasAttrs, HasTokens, Unsafe, Visibility, VisibilityKind};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Ordering;
use rustc_errors::PResult;
use rustc_errors::{
Applicability, DiagnosticBuilder, ErrorGuaranteed, FatalError, IntoDiagnostic, MultiSpan,
Expand Down Expand Up @@ -1540,8 +1541,10 @@ pub(crate) fn make_unclosed_delims_error(
}

pub fn emit_unclosed_delims(unclosed_delims: &mut Vec<UnmatchedDelim>, sess: &ParseSess) {
*sess.reached_eof.borrow_mut() |=
unclosed_delims.iter().any(|unmatched_delim| unmatched_delim.found_delim.is_none());
let _ = sess.reached_eof.fetch_or(
unclosed_delims.iter().any(|unmatched_delim| unmatched_delim.found_delim.is_none()),
Ordering::Relaxed,
);
for unmatched in unclosed_delims.drain(..) {
if let Some(mut e) = make_unclosed_delims_error(unmatched, sess) {
e.emit();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ fn sigpipe(tcx: TyCtxt<'_>, def_id: DefId) -> u8 {

fn no_main_err(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) {
let sp = tcx.def_span(CRATE_DEF_ID);
if *tcx.sess.parse_sess.reached_eof.borrow() {
if tcx.sess.parse_sess.reached_eof.load(rustc_data_structures::sync::Ordering::Relaxed) {
// There's an unclosed brace that made the parser reach `Eof`, we shouldn't complain about
// the missing `fn main()` then as it might have been hidden inside an unclosed block.
tcx.sess.delay_span_bug(sp, "`main` not found, but expected unclosed brace error");
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_query_system/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ use rustc_data_structures::sync::Lock;

use std::hash::Hash;

#[derive(Clone)]
pub struct Cache<Key, Value> {
hashmap: Lock<FxHashMap<Key, WithDepNode<Value>>>,
}

impl<Key: Clone, Value: Clone> Clone for Cache<Key, Value> {
fn clone(&self) -> Self {
Self { hashmap: Lock::new(self.hashmap.borrow().clone()) }
}
}

impl<Key, Value> Default for Cache<Key, Value> {
fn default() -> Self {
Self { hashmap: Default::default() }
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_session/src/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::search_paths::PathKind;
use crate::utils::NativeLibKind;
use crate::Session;
use rustc_ast as ast;
use rustc_data_structures::sync::{self, AppendOnlyVec, MetadataRef, RwLock};
use rustc_data_structures::sync::{self, AppendOnlyIndexVec, MetadataRef, RwLock};
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, StableCrateId, LOCAL_CRATE};
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, Definitions};
use rustc_span::hygiene::{ExpnHash, ExpnId};
Expand Down Expand Up @@ -257,6 +257,6 @@ pub type CrateStoreDyn = dyn CrateStore + sync::Sync + sync::Send;
pub struct Untracked {
pub cstore: RwLock<Box<CrateStoreDyn>>,
/// Reference span for definitions.
pub source_span: AppendOnlyVec<LocalDefId, Span>,
pub source_span: AppendOnlyIndexVec<LocalDefId, Span>,
pub definitions: RwLock<Definitions>,
}
22 changes: 11 additions & 11 deletions compiler/rustc_session/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::lint::{
};
use rustc_ast::node_id::NodeId;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_data_structures::sync::{Lock, Lrc};
use rustc_data_structures::sync::{AppendOnlyVec, AtomicBool, Lock, Lrc};
use rustc_errors::{emitter::SilentEmitter, ColorConfig, Handler};
use rustc_errors::{
fallback_fluent_bundle, Diagnostic, DiagnosticBuilder, DiagnosticId, DiagnosticMessage,
Expand Down Expand Up @@ -194,7 +194,7 @@ pub struct ParseSess {
pub edition: Edition,
/// Places where raw identifiers were used. This is used to avoid complaining about idents
/// clashing with keywords in new editions.
pub raw_identifier_spans: Lock<Vec<Span>>,
pub raw_identifier_spans: AppendOnlyVec<Span>,
/// Places where identifiers that contain invalid Unicode codepoints but that look like they
/// should be. Useful to avoid bad tokenization when encountering emoji. We group them to
/// provide a single error per unique incorrect identifier.
Expand All @@ -208,7 +208,7 @@ pub struct ParseSess {
pub gated_spans: GatedSpans,
pub symbol_gallery: SymbolGallery,
/// The parser has reached `Eof` due to an unclosed brace. Used to silence unnecessary errors.
pub reached_eof: Lock<bool>,
pub reached_eof: AtomicBool,
/// Environment variables accessed during the build and their values when they exist.
pub env_depinfo: Lock<FxHashSet<(Symbol, Option<Symbol>)>>,
/// File paths accessed during the build.
Expand All @@ -219,7 +219,7 @@ pub struct ParseSess {
pub assume_incomplete_release: bool,
/// Spans passed to `proc_macro::quote_span`. Each span has a numerical
/// identifier represented by its position in the vector.
pub proc_macro_quoted_spans: Lock<Vec<Span>>,
pub proc_macro_quoted_spans: AppendOnlyVec<Span>,
/// Used to generate new `AttrId`s. Every `AttrId` is unique.
pub attr_id_generator: AttrIdGenerator,
}
Expand Down Expand Up @@ -247,14 +247,14 @@ impl ParseSess {
config: FxIndexSet::default(),
check_config: CrateCheckConfig::default(),
edition: ExpnId::root().expn_data().edition,
raw_identifier_spans: Lock::new(Vec::new()),
raw_identifier_spans: Default::default(),
bad_unicode_identifiers: Lock::new(Default::default()),
source_map,
buffered_lints: Lock::new(vec![]),
ambiguous_block_expr_parse: Lock::new(FxHashMap::default()),
gated_spans: GatedSpans::default(),
symbol_gallery: SymbolGallery::default(),
reached_eof: Lock::new(false),
reached_eof: AtomicBool::new(false),
env_depinfo: Default::default(),
file_depinfo: Default::default(),
type_ascription_path_suggestions: Default::default(),
Expand Down Expand Up @@ -324,13 +324,13 @@ impl ParseSess {
}

pub fn save_proc_macro_span(&self, span: Span) -> usize {
let mut spans = self.proc_macro_quoted_spans.lock();
spans.push(span);
return spans.len() - 1;
self.proc_macro_quoted_spans.push(span)
}

pub fn proc_macro_quoted_spans(&self) -> Vec<Span> {
self.proc_macro_quoted_spans.lock().clone()
pub fn proc_macro_quoted_spans(&self) -> impl Iterator<Item = (usize, Span)> + '_ {
// This is equivalent to `.iter().copied().enumerate()`, but that isn't possible for
// AppendOnlyVec, so we resort to this scheme.
self.proc_macro_quoted_spans.iter_enumerated()
}

#[track_caller]
Expand Down
Loading