Skip to content

Commit bd4a96a

Browse files
borsbors
committedFeb 22, 2023
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
2 parents f9216b7 + decfb4d commit bd4a96a

File tree

12 files changed

+79
-36
lines changed

12 files changed

+79
-36
lines changed
 

Diff for: ‎Cargo.lock

+10
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,15 @@ dependencies = [
13781378
"zeroize",
13791379
]
13801380

1381+
[[package]]
1382+
name = "elsa"
1383+
version = "1.8.0"
1384+
source = "registry+https://github.com/rust-lang/crates.io-index"
1385+
checksum = "f74077c3c3aedb99a2683919698285596662518ea13e5eedcf8bdd43b0d0453b"
1386+
dependencies = [
1387+
"stable_deref_trait",
1388+
]
1389+
13811390
[[package]]
13821391
name = "ena"
13831392
version = "0.14.0"
@@ -3884,6 +3893,7 @@ dependencies = [
38843893
"arrayvec 0.7.0",
38853894
"bitflags",
38863895
"cfg-if",
3896+
"elsa",
38873897
"ena",
38883898
"indexmap",
38893899
"jobserver",

Diff for: ‎compiler/rustc_data_structures/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ stacker = "0.1.15"
3131
tempfile = "3.2"
3232
thin-vec = "0.2.12"
3333
tracing = "0.1"
34+
elsa = "1.8"
3435

3536
[dependencies.parking_lot]
3637
version = "0.11"

Diff for: ‎compiler/rustc_data_structures/src/sync.rs

+4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};
2626
pub use std::sync::atomic::Ordering;
2727
pub use std::sync::atomic::Ordering::SeqCst;
2828

29+
pub use vec::AppendOnlyVec;
30+
31+
mod vec;
32+
2933
cfg_if! {
3034
if #[cfg(not(parallel_compiler))] {
3135
pub auto trait Send {}

Diff for: ‎compiler/rustc_data_structures/src/sync/vec.rs

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
use std::marker::PhantomData;
2+
3+
use rustc_index::vec::Idx;
4+
5+
pub struct AppendOnlyVec<I: Idx, T: Copy> {
6+
#[cfg(not(parallel_compiler))]
7+
vec: elsa::vec::FrozenVec<T>,
8+
#[cfg(parallel_compiler)]
9+
vec: elsa::sync::LockFreeFrozenVec<T>,
10+
_marker: PhantomData<fn(&I)>,
11+
}
12+
13+
impl<I: Idx, T: Copy> AppendOnlyVec<I, T> {
14+
pub fn new() -> Self {
15+
Self {
16+
#[cfg(not(parallel_compiler))]
17+
vec: elsa::vec::FrozenVec::new(),
18+
#[cfg(parallel_compiler)]
19+
vec: elsa::sync::LockFreeFrozenVec::new(),
20+
_marker: PhantomData,
21+
}
22+
}
23+
24+
pub fn push(&self, val: T) -> I {
25+
#[cfg(not(parallel_compiler))]
26+
let i = self.vec.len();
27+
#[cfg(not(parallel_compiler))]
28+
self.vec.push(val);
29+
#[cfg(parallel_compiler)]
30+
let i = self.vec.push(val);
31+
I::new(i)
32+
}
33+
34+
pub fn get(&self, i: I) -> Option<T> {
35+
let i = i.index();
36+
#[cfg(not(parallel_compiler))]
37+
return self.vec.get_copy(i);
38+
#[cfg(parallel_compiler)]
39+
return self.vec.get(i);
40+
}
41+
}

Diff for: ‎compiler/rustc_interface/src/queries.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ use rustc_codegen_ssa::traits::CodegenBackend;
77
use rustc_codegen_ssa::CodegenResults;
88
use rustc_data_structures::steal::Steal;
99
use rustc_data_structures::svh::Svh;
10-
use rustc_data_structures::sync::{Lrc, OnceCell, RwLock, WorkerLocal};
10+
use rustc_data_structures::sync::{AppendOnlyVec, Lrc, OnceCell, RwLock, WorkerLocal};
1111
use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
1212
use rustc_hir::definitions::Definitions;
1313
use rustc_incremental::DepGraphFuture;
14-
use rustc_index::vec::IndexVec;
1514
use rustc_lint::LintStore;
1615
use rustc_metadata::creader::CStore;
1716
use rustc_middle::arena::Arena;
@@ -195,10 +194,9 @@ impl<'tcx> Queries<'tcx> {
195194

196195
let cstore = RwLock::new(Box::new(CStore::new(sess)) as _);
197196
let definitions = RwLock::new(Definitions::new(sess.local_stable_crate_id()));
198-
let mut source_span = IndexVec::default();
197+
let source_span = AppendOnlyVec::new();
199198
let _id = source_span.push(krate.spans.inner_span);
200199
debug_assert_eq!(_id, CRATE_DEF_ID);
201-
let source_span = RwLock::new(source_span);
202200
let untracked = Untracked { cstore, source_span, definitions };
203201

204202
let qcx = passes::create_global_ctxt(

Diff for: ‎compiler/rustc_middle/src/ty/context.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1030,7 +1030,7 @@ impl<'tcx> TyCtxt<'tcx> {
10301030
/// system if the result is otherwise tracked through queries
10311031
#[inline]
10321032
pub fn source_span_untracked(self, def_id: LocalDefId) -> Span {
1033-
self.untracked.source_span.read().get(def_id).copied().unwrap_or(DUMMY_SP)
1033+
self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP)
10341034
}
10351035

10361036
#[inline(always)]
@@ -2525,6 +2525,5 @@ pub fn provide(providers: &mut ty::query::Providers) {
25252525
// We want to check if the panic handler was defined in this crate
25262526
tcx.lang_items().panic_impl().map_or(false, |did| did.is_local())
25272527
};
2528-
providers.source_span =
2529-
|tcx, def_id| tcx.untracked.source_span.read().get(def_id).copied().unwrap_or(DUMMY_SP);
2528+
providers.source_span = |tcx, def_id| tcx.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP);
25302529
}

Diff for: ‎compiler/rustc_query_system/src/ich/hcx.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
146146

147147
#[inline]
148148
fn def_span(&self, def_id: LocalDefId) -> Span {
149-
*self.untracked.source_span.read().get(def_id).unwrap_or(&DUMMY_SP)
149+
self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP)
150150
}
151151

152152
#[inline]

Diff for: ‎compiler/rustc_resolve/src/diagnostics.rs

+12-19
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,10 @@ use rustc_errors::{struct_span_err, SuggestionStyle};
1212
use rustc_feature::BUILTIN_ATTRIBUTES;
1313
use rustc_hir::def::Namespace::{self, *};
1414
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind, PerNS};
15-
use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
15+
use rustc_hir::def_id::{DefId, CRATE_DEF_ID, LOCAL_CRATE};
1616
use rustc_hir::PrimTy;
17-
use rustc_index::vec::IndexVec;
1817
use rustc_middle::bug;
19-
use rustc_middle::ty::DefIdTree;
18+
use rustc_middle::ty::{DefIdTree, TyCtxt};
2019
use rustc_session::lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE;
2120
use rustc_session::lint::builtin::MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS;
2221
use rustc_session::lint::BuiltinLintDiagnostics;
@@ -154,8 +153,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
154153

155154
if !candidates.is_empty() {
156155
show_candidates(
157-
&self.tcx.sess,
158-
&self.tcx.untracked().source_span.read(),
156+
self.tcx,
159157
&mut err,
160158
span,
161159
&candidates,
@@ -687,8 +685,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
687685
err.span_help(span, &help_msg);
688686
}
689687
show_candidates(
690-
&self.tcx.sess,
691-
&self.tcx.untracked().source_span.read(),
688+
self.tcx,
692689
&mut err,
693690
Some(span),
694691
&import_suggestions,
@@ -1352,8 +1349,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13521349
let import_suggestions =
13531350
self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
13541351
show_candidates(
1355-
&self.tcx.sess,
1356-
&self.tcx.untracked().source_span.read(),
1352+
self.tcx,
13571353
err,
13581354
None,
13591355
&import_suggestions,
@@ -2358,8 +2354,7 @@ pub(crate) enum DiagnosticMode {
23582354
}
23592355

23602356
pub(crate) fn import_candidates(
2361-
session: &Session,
2362-
source_span: &IndexVec<LocalDefId, Span>,
2357+
tcx: TyCtxt<'_>,
23632358
err: &mut Diagnostic,
23642359
// This is `None` if all placement locations are inside expansions
23652360
use_placement_span: Option<Span>,
@@ -2368,8 +2363,7 @@ pub(crate) fn import_candidates(
23682363
append: &str,
23692364
) {
23702365
show_candidates(
2371-
session,
2372-
source_span,
2366+
tcx,
23732367
err,
23742368
use_placement_span,
23752369
candidates,
@@ -2385,8 +2379,7 @@ pub(crate) fn import_candidates(
23852379
/// entities with that name in all crates. This method allows outputting the
23862380
/// results of this search in a programmer-friendly way
23872381
fn show_candidates(
2388-
session: &Session,
2389-
source_span: &IndexVec<LocalDefId, Span>,
2382+
tcx: TyCtxt<'_>,
23902383
err: &mut Diagnostic,
23912384
// This is `None` if all placement locations are inside expansions
23922385
use_placement_span: Option<Span>,
@@ -2511,8 +2504,8 @@ fn show_candidates(
25112504
);
25122505

25132506
if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
2514-
let span = source_span[local_def_id];
2515-
let span = session.source_map().guess_head_span(span);
2507+
let span = tcx.source_span(local_def_id);
2508+
let span = tcx.sess.source_map().guess_head_span(span);
25162509
let mut multi_span = MultiSpan::from_span(span);
25172510
multi_span.push_span_label(span, "not accessible");
25182511
err.span_note(multi_span, &msg);
@@ -2542,8 +2535,8 @@ fn show_candidates(
25422535
let mut spans = Vec::new();
25432536
for (name, _, def_id, _) in &inaccessible_path_strings {
25442537
if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
2545-
let span = source_span[local_def_id];
2546-
let span = session.source_map().guess_head_span(span);
2538+
let span = tcx.source_span(local_def_id);
2539+
let span = tcx.sess.source_map().guess_head_span(span);
25472540
spans.push((name, span));
25482541
} else {
25492542
if !has_colon {

Diff for: ‎compiler/rustc_resolve/src/imports.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,7 @@ impl<'a, 'b, 'tcx> ImportResolver<'a, 'b, 'tcx> {
548548
if let Some(candidates) = &err.candidates {
549549
match &import.kind {
550550
ImportKind::Single { nested: false, source, target, .. } => import_candidates(
551-
self.r.tcx.sess,
552-
&self.r.tcx.untracked().source_span.read(),
551+
self.r.tcx,
553552
&mut diag,
554553
Some(err.span),
555554
&candidates,
@@ -561,8 +560,7 @@ impl<'a, 'b, 'tcx> ImportResolver<'a, 'b, 'tcx> {
561560
),
562561
ImportKind::Single { nested: true, source, target, .. } => {
563562
import_candidates(
564-
self.r.tcx.sess,
565-
&self.r.tcx.untracked().source_span.read(),
563+
self.r.tcx,
566564
&mut diag,
567565
None,
568566
&candidates,

Diff for: ‎compiler/rustc_resolve/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ impl<'tcx> Resolver<'_, 'tcx> {
11561156

11571157
// A relative span's parent must be an absolute span.
11581158
debug_assert_eq!(span.data_untracked().parent, None);
1159-
let _id = self.tcx.untracked().source_span.write().push(span);
1159+
let _id = self.tcx.untracked().source_span.push(span);
11601160
debug_assert_eq!(_id, def_id);
11611161

11621162
// Some things for which we allocate `LocalDefId`s don't correspond to

Diff for: ‎compiler/rustc_session/src/cstore.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ use crate::search_paths::PathKind;
66
use crate::utils::NativeLibKind;
77
use crate::Session;
88
use rustc_ast as ast;
9-
use rustc_data_structures::sync::{self, MetadataRef, RwLock};
9+
use rustc_data_structures::sync::{self, AppendOnlyVec, MetadataRef, RwLock};
1010
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, StableCrateId, LOCAL_CRATE};
1111
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, Definitions};
12-
use rustc_index::vec::IndexVec;
1312
use rustc_span::hygiene::{ExpnHash, ExpnId};
1413
use rustc_span::symbol::Symbol;
1514
use rustc_span::Span;
@@ -252,10 +251,9 @@ pub trait CrateStore: std::fmt::Debug {
252251

253252
pub type CrateStoreDyn = dyn CrateStore + sync::Sync + sync::Send;
254253

255-
#[derive(Debug)]
256254
pub struct Untracked {
257255
pub cstore: RwLock<Box<CrateStoreDyn>>,
258256
/// Reference span for definitions.
259-
pub source_span: RwLock<IndexVec<LocalDefId, Span>>,
257+
pub source_span: AppendOnlyVec<LocalDefId, Span>,
260258
pub definitions: RwLock<Definitions>,
261259
}

Diff for: ‎src/tools/tidy/src/deps.rs

+1
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[
120120
"dissimilar",
121121
"dlmalloc",
122122
"either",
123+
"elsa",
123124
"ena",
124125
"expect-test",
125126
"fallible-iterator", // dependency of `thorin`

0 commit comments

Comments
 (0)
Please sign in to comment.