Auto merge of rust-lang#108300 - oli-obk:elsa, r=eholk

Use a lock-free datastructure for source_span follow up to the perf regression in rust-lang#105462 The main regression is likely the CStore, but let's evaluate the perf impact of this on its own
rust-lang-ci · Feb 22, 2023 · bd4a96a · bd4a96a
2 parents f9216b7 + decfb4d
commit bd4a96a
Show file tree

Hide file tree

Showing 12 changed files with 79 additions and 36 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -1378,6 +1378,15 @@ dependencies = [
  "zeroize",
 ]
 
+[[package]]
+name = "elsa"
+version = "1.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f74077c3c3aedb99a2683919698285596662518ea13e5eedcf8bdd43b0d0453b"
+dependencies = [
+ "stable_deref_trait",
+]
+
 [[package]]
 name = "ena"
 version = "0.14.0"
@@ -3884,6 +3893,7 @@ dependencies = [
  "arrayvec 0.7.0",
  "bitflags",
  "cfg-if",
+ "elsa",
  "ena",
  "indexmap",
  "jobserver",

diff --git a/compiler/rustc_data_structures/Cargo.toml b/compiler/rustc_data_structures/Cargo.toml
@@ -31,6 +31,7 @@ stacker = "0.1.15"
 tempfile = "3.2"
 thin-vec = "0.2.12"
 tracing = "0.1"
+elsa = "1.8"
 
 [dependencies.parking_lot]
 version = "0.11"

diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs
@@ -26,6 +26,10 @@ 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;
+
+mod vec;
+
 cfg_if! {
     if #[cfg(not(parallel_compiler))] {
         pub auto trait Send {}

diff --git a/compiler/rustc_data_structures/src/sync/vec.rs b/compiler/rustc_data_structures/src/sync/vec.rs
@@ -0,0 +1,41 @@
+use std::marker::PhantomData;
+
+use rustc_index::vec::Idx;
+
+pub struct AppendOnlyVec<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> {
+    pub fn new() -> Self {
+        Self {
+            #[cfg(not(parallel_compiler))]
+            vec: elsa::vec::FrozenVec::new(),
+            #[cfg(parallel_compiler)]
+            vec: elsa::sync::LockFreeFrozenVec::new(),
+            _marker: PhantomData,
+        }
+    }
+
+    pub fn push(&self, val: T) -> I {
+        #[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::new(i)
+    }
+
+    pub fn get(&self, i: I) -> Option<T> {
+        let i = i.index();
+        #[cfg(not(parallel_compiler))]
+        return self.vec.get_copy(i);
+        #[cfg(parallel_compiler)]
+        return self.vec.get(i);
+    }
+}
diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs
@@ -7,11 +7,10 @@ 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::{Lrc, OnceCell, RwLock, WorkerLocal};
+use rustc_data_structures::sync::{AppendOnlyVec, Lrc, OnceCell, RwLock, WorkerLocal};
 use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_hir::definitions::Definitions;
 use rustc_incremental::DepGraphFuture;
-use rustc_index::vec::IndexVec;
 use rustc_lint::LintStore;
 use rustc_metadata::creader::CStore;
 use rustc_middle::arena::Arena;
@@ -195,10 +194,9 @@ 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 mut source_span = IndexVec::default();
+            let source_span = AppendOnlyVec::new();
             let _id = source_span.push(krate.spans.inner_span);
             debug_assert_eq!(_id, CRATE_DEF_ID);
-            let source_span = RwLock::new(source_span);
             let untracked = Untracked { cstore, source_span, definitions };
 
             let qcx = passes::create_global_ctxt(

diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
@@ -1030,7 +1030,7 @@ impl<'tcx> TyCtxt<'tcx> {
     /// system if the result is otherwise tracked through queries
     #[inline]
     pub fn source_span_untracked(self, def_id: LocalDefId) -> Span {
-        self.untracked.source_span.read().get(def_id).copied().unwrap_or(DUMMY_SP)
+        self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP)
     }
 
     #[inline(always)]
@@ -2525,6 +2525,5 @@ pub fn provide(providers: &mut ty::query::Providers) {
         // We want to check if the panic handler was defined in this crate
         tcx.lang_items().panic_impl().map_or(false, |did| did.is_local())
     };
-    providers.source_span =
-        |tcx, def_id| tcx.untracked.source_span.read().get(def_id).copied().unwrap_or(DUMMY_SP);
+    providers.source_span = |tcx, def_id| tcx.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP);
 }
diff --git a/compiler/rustc_query_system/src/ich/hcx.rs b/compiler/rustc_query_system/src/ich/hcx.rs
@@ -146,7 +146,7 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
 
     #[inline]
     fn def_span(&self, def_id: LocalDefId) -> Span {
-        *self.untracked.source_span.read().get(def_id).unwrap_or(&DUMMY_SP)
+        self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP)
     }
 
     #[inline]

diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
@@ -12,11 +12,10 @@ use rustc_errors::{struct_span_err, SuggestionStyle};
 use rustc_feature::BUILTIN_ATTRIBUTES;
 use rustc_hir::def::Namespace::{self, *};
 use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind, PerNS};
-use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
+use rustc_hir::def_id::{DefId, CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_hir::PrimTy;
-use rustc_index::vec::IndexVec;
 use rustc_middle::bug;
-use rustc_middle::ty::DefIdTree;
+use rustc_middle::ty::{DefIdTree, TyCtxt};
 use rustc_session::lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE;
 use rustc_session::lint::builtin::MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS;
 use rustc_session::lint::BuiltinLintDiagnostics;
@@ -154,8 +153,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
             if !candidates.is_empty() {
                 show_candidates(
-                    &self.tcx.sess,
-                    &self.tcx.untracked().source_span.read(),
+                    self.tcx,
                     &mut err,
                     span,
                     &candidates,
@@ -687,8 +685,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         err.span_help(span, &help_msg);
                     }
                     show_candidates(
-                        &self.tcx.sess,
-                        &self.tcx.untracked().source_span.read(),
+                        self.tcx,
                         &mut err,
                         Some(span),
                         &import_suggestions,
@@ -1352,8 +1349,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         let import_suggestions =
             self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
         show_candidates(
-            &self.tcx.sess,
-            &self.tcx.untracked().source_span.read(),
+            self.tcx,
             err,
             None,
             &import_suggestions,
@@ -2358,8 +2354,7 @@ pub(crate) enum DiagnosticMode {
 }
 
 pub(crate) fn import_candidates(
-    session: &Session,
-    source_span: &IndexVec<LocalDefId, Span>,
+    tcx: TyCtxt<'_>,
     err: &mut Diagnostic,
     // This is `None` if all placement locations are inside expansions
     use_placement_span: Option<Span>,
@@ -2368,8 +2363,7 @@ pub(crate) fn import_candidates(
     append: &str,
 ) {
     show_candidates(
-        session,
-        source_span,
+        tcx,
         err,
         use_placement_span,
         candidates,
@@ -2385,8 +2379,7 @@ pub(crate) fn import_candidates(
 /// entities with that name in all crates. This method allows outputting the
 /// results of this search in a programmer-friendly way
 fn show_candidates(
-    session: &Session,
-    source_span: &IndexVec<LocalDefId, Span>,
+    tcx: TyCtxt<'_>,
     err: &mut Diagnostic,
     // This is `None` if all placement locations are inside expansions
     use_placement_span: Option<Span>,
@@ -2511,8 +2504,8 @@ fn show_candidates(
             );
 
             if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
-                let span = source_span[local_def_id];
-                let span = session.source_map().guess_head_span(span);
+                let span = tcx.source_span(local_def_id);
+                let span = tcx.sess.source_map().guess_head_span(span);
                 let mut multi_span = MultiSpan::from_span(span);
                 multi_span.push_span_label(span, "not accessible");
                 err.span_note(multi_span, &msg);
@@ -2542,8 +2535,8 @@ fn show_candidates(
             let mut spans = Vec::new();
             for (name, _, def_id, _) in &inaccessible_path_strings {
                 if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
-                    let span = source_span[local_def_id];
-                    let span = session.source_map().guess_head_span(span);
+                    let span = tcx.source_span(local_def_id);
+                    let span = tcx.sess.source_map().guess_head_span(span);
                     spans.push((name, span));
                 } else {
                     if !has_colon {

diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -548,8 +548,7 @@ impl<'a, 'b, 'tcx> ImportResolver<'a, 'b, 'tcx> {
             if let Some(candidates) = &err.candidates {
                 match &import.kind {
                     ImportKind::Single { nested: false, source, target, .. } => import_candidates(
-                        self.r.tcx.sess,
-                        &self.r.tcx.untracked().source_span.read(),
+                        self.r.tcx,
                         &mut diag,
                         Some(err.span),
                         &candidates,
@@ -561,8 +560,7 @@ impl<'a, 'b, 'tcx> ImportResolver<'a, 'b, 'tcx> {
                     ),
                     ImportKind::Single { nested: true, source, target, .. } => {
                         import_candidates(
-                            self.r.tcx.sess,
-                            &self.r.tcx.untracked().source_span.read(),
+                            self.r.tcx,
                             &mut diag,
                             None,
                             &candidates,

diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -1156,7 +1156,7 @@ impl<'tcx> Resolver<'_, 'tcx> {
 
         // A relative span's parent must be an absolute span.
         debug_assert_eq!(span.data_untracked().parent, None);
-        let _id = self.tcx.untracked().source_span.write().push(span);
+        let _id = self.tcx.untracked().source_span.push(span);
         debug_assert_eq!(_id, def_id);
 
         // Some things for which we allocate `LocalDefId`s don't correspond to

diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs
@@ -6,10 +6,9 @@ use crate::search_paths::PathKind;
 use crate::utils::NativeLibKind;
 use crate::Session;
 use rustc_ast as ast;
-use rustc_data_structures::sync::{self, MetadataRef, RwLock};
+use rustc_data_structures::sync::{self, AppendOnlyVec, MetadataRef, RwLock};
 use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, StableCrateId, LOCAL_CRATE};
 use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, Definitions};
-use rustc_index::vec::IndexVec;
 use rustc_span::hygiene::{ExpnHash, ExpnId};
 use rustc_span::symbol::Symbol;
 use rustc_span::Span;
@@ -252,10 +251,9 @@ pub trait CrateStore: std::fmt::Debug {
 
 pub type CrateStoreDyn = dyn CrateStore + sync::Sync + sync::Send;
 
-#[derive(Debug)]
 pub struct Untracked {
     pub cstore: RwLock<Box<CrateStoreDyn>>,
     /// Reference span for definitions.
-    pub source_span: RwLock<IndexVec<LocalDefId, Span>>,
+    pub source_span: AppendOnlyVec<LocalDefId, Span>,
     pub definitions: RwLock<Definitions>,
 }
diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs
@@ -120,6 +120,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[
     "dissimilar",
     "dlmalloc",
     "either",
+    "elsa",
     "ena",
     "expect-test",
     "fallible-iterator", // dependency of `thorin`