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

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.

Author: bors

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",
 ]

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"

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>);
 

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
+    }
+}

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 };

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;
         }
 

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);
             }
         }

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 {

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);
             }

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 }
     }