Auto merge of #109117 - oli-obk:locks, r=michaelwoerister

bors · bors · commit 383c1d729ead · 2023-04-05T10:38:02.000Z
Avoid a few locks

We can use atomics or datastructures tuned for specific access patterns instead of locks. This may be an improvement for parallel rustc, but it's mostly a cleanup making various datastructures only usable in the way they are used right now (append data, never mutate), instead of having a general purpose lock.
diff --git a/Cargo.lock b/Cargo.lock
@@ -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",
 ]
diff --git a/compiler/rustc_data_structures/Cargo.toml b/compiler/rustc_data_structures/Cargo.toml
@@ -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"
diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs
@@ -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;
 
@@ -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,
@@ -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>);
 
diff --git a/compiler/rustc_data_structures/src/sync/vec.rs b/compiler/rustc_data_structures/src/sync/vec.rs
@@ -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))]
@@ -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
+    }
+}
diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs
@@ -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;
@@ -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 };
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -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;
         }
 
diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs
@@ -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);
                 }
@@ -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);
             }
         }
diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs
@@ -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};
@@ -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.
@@ -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.
@@ -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 {
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -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);
             }
diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs
@@ -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 }
     }
diff --git a/compiler/rustc_parse/src/lexer/mod.rs b/compiler/rustc_parse/src/lexer/mod.rs
@@ -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 => {
diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs
@@ -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,
@@ -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();
diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs
@@ -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");
diff --git a/compiler/rustc_query_system/src/cache.rs b/compiler/rustc_query_system/src/cache.rs
@@ -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() }
diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs
@@ -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};
@@ -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>,
 }
diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs
@@ -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,
@@ -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.
@@ -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.
@@ -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,
 }
@@ -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(),
@@ -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]
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs

Original file line number	Diff line number	Diff line change
`@@ -1947,7 +1947,7 @@ impl KeywordIdents {`
`1947`	`1947`	`};`
`1948`	`1948`
`1949`	`1949`	// Don't lint `r#foo`.
`1950`		`- if cx.sess().parse_sess.raw_identifier_spans.borrow().contains(&ident.span) {`
	`1950`	`+ if cx.sess().parse_sess.raw_identifier_spans.contains(ident.span) {`
`1951`	`1951`	`return;`
`1952`	`1952`	`}`
`1953`	`1953`
Original file line number	Diff line number	Diff line change
`@@ -185,7 +185,7 @@ impl CStore {`
`185`	`185`	`fn push_dependencies_in_postorder(&self, deps: &mut Vec<CrateNum>, cnum: CrateNum) {`
`186`	`186`	`if !deps.contains(&cnum) {`
`187`	`187`	`let data = self.get_crate_data(cnum);`
`188`		`- for &dep in data.dependencies().iter() {`
	`188`	`+ for dep in data.dependencies() {`
`189`	`189`	`if dep != cnum {`
`190`	`190`	`self.push_dependencies_in_postorder(deps, dep);`
`191`	`191`	`}`
`@@ -605,7 +605,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {`
`605`	`605`	`if cmeta.update_extern_crate(extern_crate) {`
`606`	`606`	`// Propagate the extern crate info to dependencies if it was updated.`
`607`	`607`	`let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate };`
`608`		`- for &dep_cnum in cmeta.dependencies().iter() {`
	`608`	`+ for dep_cnum in cmeta.dependencies() {`
`609`	`609`	`self.update_extern_crate(dep_cnum, extern_crate);`
`610`	`610`	`}`
`611`	`611`	`}`
Original file line number	Diff line number	Diff line change
`@@ -263,7 +263,8 @@ impl AllocDecodingState {`
`263`	`263`	`}`
`264`	`264`
`265`	`265`	`pub fn new(data_offsets: Vec<u32>) -> Self {`
`266`		`- let decoding_state = vec![Lock::new(State::Empty); data_offsets.len()];`
	`266`	`+ let decoding_state =`
	`267`	`+ std::iter::repeat_with(\|\| Lock::new(State::Empty)).take(data_offsets.len()).collect();`
`267`	`268`
`268`	`269`	`Self { decoding_state, data_offsets }`
`269`	`270`	`}`