Skip to content

Commit 6d32b29

Browse files
borsbors
committed
Auto merge of #114894 - Zoxc:sharded-cfg-cleanup2, r=cjgillot
Remove conditional use of `Sharded` from query state `Sharded` is already a zero cost abstraction, so it shouldn't affect the performance of the single thread compiler if LLVM does its job. r? `@cjgillot`
2 parents 0b84f18 + f458b11 commit 6d32b29

File tree

4 files changed

+48
-73
lines changed

4 files changed

+48
-73
lines changed

‎compiler/rustc_data_structures/src/sharded.rs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ use crate::fx::{FxHashMap, FxHasher};
22
#[cfg(parallel_compiler)]
33
use crate::sync::{is_dyn_thread_safe, CacheAligned};
44
use crate::sync::{Lock, LockGuard};
5+
#[cfg(parallel_compiler)]
6+
use itertools::Either;
57
use std::borrow::Borrow;
68
use std::collections::hash_map::RawEntryMut;
79
use std::hash::{Hash, Hasher};
10+
use std::iter;
811
use std::mem;
912

1013
// 32 shards is sufficient to reduce contention on an 8-core Ryzen 7 1700,
@@ -70,19 +73,27 @@ impl<T> Sharded<T> {
7073
}
7174
}
7275

73-
pub fn lock_shards(&self) -> Vec<LockGuard<'_, T>> {
76+
#[inline]
77+
pub fn lock_shards(&self) -> impl Iterator<Item = LockGuard<'_, T>> {
7478
match self {
75-
Self::Single(single) => vec![single.lock()],
79+
#[cfg(not(parallel_compiler))]
80+
Self::Single(single) => iter::once(single.lock()),
81+
#[cfg(parallel_compiler)]
82+
Self::Single(single) => Either::Left(iter::once(single.lock())),
7683
#[cfg(parallel_compiler)]
77-
Self::Shards(shards) => shards.iter().map(|shard| shard.0.lock()).collect(),
84+
Self::Shards(shards) => Either::Right(shards.iter().map(|shard| shard.0.lock())),
7885
}
7986
}
8087

81-
pub fn try_lock_shards(&self) -> Option<Vec<LockGuard<'_, T>>> {
88+
#[inline]
89+
pub fn try_lock_shards(&self) -> impl Iterator<Item = Option<LockGuard<'_, T>>> {
8290
match self {
83-
Self::Single(single) => Some(vec![single.try_lock()?]),
91+
#[cfg(not(parallel_compiler))]
92+
Self::Single(single) => iter::once(single.try_lock()),
93+
#[cfg(parallel_compiler)]
94+
Self::Single(single) => Either::Left(iter::once(single.try_lock())),
8495
#[cfg(parallel_compiler)]
85-
Self::Shards(shards) => shards.iter().map(|shard| shard.0.try_lock()).collect(),
96+
Self::Shards(shards) => Either::Right(shards.iter().map(|shard| shard.0.try_lock())),
8697
}
8798
}
8899
}
@@ -101,7 +112,7 @@ pub type ShardedHashMap<K, V> = Sharded<FxHashMap<K, V>>;
101112

102113
impl<K: Eq, V> ShardedHashMap<K, V> {
103114
pub fn len(&self) -> usize {
104-
self.lock_shards().iter().map(|shard| shard.len()).sum()
115+
self.lock_shards().map(|shard| shard.len()).sum()
105116
}
106117
}
107118

‎compiler/rustc_middle/src/ty/context.rs‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,25 +1296,26 @@ macro_rules! sty_debug_print {
12961296
};
12971297
$(let mut $variant = total;)*
12981298

1299-
let shards = tcx.interners.type_.lock_shards();
1300-
let types = shards.iter().flat_map(|shard| shard.keys());
1301-
for &InternedInSet(t) in types {
1302-
let variant = match t.internee {
1303-
ty::Bool | ty::Char | ty::Int(..) | ty::Uint(..) |
1304-
ty::Float(..) | ty::Str | ty::Never => continue,
1305-
ty::Error(_) => /* unimportant */ continue,
1306-
$(ty::$variant(..) => &mut $variant,)*
1307-
};
1308-
let lt = t.flags.intersects(ty::TypeFlags::HAS_RE_INFER);
1309-
let ty = t.flags.intersects(ty::TypeFlags::HAS_TY_INFER);
1310-
let ct = t.flags.intersects(ty::TypeFlags::HAS_CT_INFER);
1311-
1312-
variant.total += 1;
1313-
total.total += 1;
1314-
if lt { total.lt_infer += 1; variant.lt_infer += 1 }
1315-
if ty { total.ty_infer += 1; variant.ty_infer += 1 }
1316-
if ct { total.ct_infer += 1; variant.ct_infer += 1 }
1317-
if lt && ty && ct { total.all_infer += 1; variant.all_infer += 1 }
1299+
for shard in tcx.interners.type_.lock_shards() {
1300+
let types = shard.keys();
1301+
for &InternedInSet(t) in types {
1302+
let variant = match t.internee {
1303+
ty::Bool | ty::Char | ty::Int(..) | ty::Uint(..) |
1304+
ty::Float(..) | ty::Str | ty::Never => continue,
1305+
ty::Error(_) => /* unimportant */ continue,
1306+
$(ty::$variant(..) => &mut $variant,)*
1307+
};
1308+
let lt = t.flags.intersects(ty::TypeFlags::HAS_RE_INFER);
1309+
let ty = t.flags.intersects(ty::TypeFlags::HAS_TY_INFER);
1310+
let ct = t.flags.intersects(ty::TypeFlags::HAS_CT_INFER);
1311+
1312+
variant.total += 1;
1313+
total.total += 1;
1314+
if lt { total.lt_infer += 1; variant.lt_infer += 1 }
1315+
if ty { total.ty_infer += 1; variant.ty_infer += 1 }
1316+
if ct { total.ct_infer += 1; variant.ct_infer += 1 }
1317+
if lt && ty && ct { total.all_infer += 1; variant.all_infer += 1 }
1318+
}
13181319
}
13191320
writeln!(fmt, "Ty interner total ty lt ct all")?;
13201321
$(writeln!(fmt, " {:18}: {uses:6} {usespc:4.1}%, \

‎compiler/rustc_query_system/src/query/caches.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ where
7070
}
7171

7272
fn iter(&self, f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex)) {
73-
let shards = self.cache.lock_shards();
74-
for shard in shards.iter() {
73+
for shard in self.cache.lock_shards() {
7574
for (k, v) in shard.iter() {
7675
f(k, &v.0, v.1);
7776
}
@@ -160,8 +159,7 @@ where
160159
}
161160

162161
fn iter(&self, f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex)) {
163-
let shards = self.cache.lock_shards();
164-
for shard in shards.iter() {
162+
for shard in self.cache.lock_shards() {
165163
for (k, v) in shard.iter_enumerated() {
166164
if let Some(v) = v {
167165
f(&k, &v.0, v.1);

‎compiler/rustc_query_system/src/query/plumbing.rs‎

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@ use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobI
1212
use crate::query::SerializedDepNodeIndex;
1313
use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
1414
use crate::HandleCycleError;
15+
#[cfg(parallel_compiler)]
16+
use rustc_data_structures::cold_path;
1517
use rustc_data_structures::fingerprint::Fingerprint;
1618
use rustc_data_structures::fx::FxHashMap;
19+
use rustc_data_structures::sharded::Sharded;
1720
use rustc_data_structures::stack::ensure_sufficient_stack;
1821
use rustc_data_structures::sync::Lock;
19-
#[cfg(parallel_compiler)]
20-
use rustc_data_structures::{cold_path, sharded::Sharded};
2122
use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed, FatalError};
2223
use rustc_span::{Span, DUMMY_SP};
2324
use std::cell::Cell;
@@ -30,10 +31,7 @@ use thin_vec::ThinVec;
3031
use super::QueryConfig;
3132

3233
pub struct QueryState<K, D: DepKind> {
33-
#[cfg(parallel_compiler)]
3434
active: Sharded<FxHashMap<K, QueryResult<D>>>,
35-
#[cfg(not(parallel_compiler))]
36-
active: Lock<FxHashMap<K, QueryResult<D>>>,
3735
}
3836

3937
/// Indicates the state of a query for a given key in a query map.
@@ -52,15 +50,7 @@ where
5250
D: DepKind,
5351
{
5452
pub fn all_inactive(&self) -> bool {
55-
#[cfg(parallel_compiler)]
56-
{
57-
let shards = self.active.lock_shards();
58-
shards.iter().all(|shard| shard.is_empty())
59-
}
60-
#[cfg(not(parallel_compiler))]
61-
{
62-
self.active.lock().is_empty()
63-
}
53+
self.active.lock_shards().all(|shard| shard.is_empty())
6454
}
6555

6656
pub fn try_collect_active_jobs<Qcx: Copy>(
@@ -71,26 +61,10 @@ where
7161
) -> Option<()> {
7262
let mut active = Vec::new();
7363

74-
#[cfg(parallel_compiler)]
75-
{
76-
// We use try_lock_shards here since we are called from the
77-
// deadlock handler, and this shouldn't be locked.
78-
let shards = self.active.try_lock_shards()?;
79-
for shard in shards.iter() {
80-
for (k, v) in shard.iter() {
81-
if let QueryResult::Started(ref job) = *v {
82-
active.push((*k, job.clone()));
83-
}
84-
}
85-
}
86-
}
87-
#[cfg(not(parallel_compiler))]
88-
{
89-
// We use try_lock here since we are called from the
90-
// deadlock handler, and this shouldn't be locked.
91-
// (FIXME: Is this relevant for non-parallel compilers? It doesn't
92-
// really hurt much.)
93-
for (k, v) in self.active.try_lock()?.iter() {
64+
// We use try_lock_shards here since we are called from the
65+
// deadlock handler, and this shouldn't be locked.
66+
for shard in self.active.try_lock_shards() {
67+
for (k, v) in shard?.iter() {
9468
if let QueryResult::Started(ref job) = *v {
9569
active.push((*k, job.clone()));
9670
}
@@ -184,10 +158,7 @@ where
184158
cache.complete(key, result, dep_node_index);
185159

186160
let job = {
187-
#[cfg(parallel_compiler)]
188161
let mut lock = state.active.get_shard_by_value(&key).lock();
189-
#[cfg(not(parallel_compiler))]
190-
let mut lock = state.active.lock();
191162
match lock.remove(&key).unwrap() {
192163
QueryResult::Started(job) => job,
193164
QueryResult::Poisoned => panic!(),
@@ -209,10 +180,7 @@ where
209180
// Poison the query so jobs waiting on it panic.
210181
let state = self.state;
211182
let job = {
212-
#[cfg(parallel_compiler)]
213183
let mut shard = state.active.get_shard_by_value(&self.key).lock();
214-
#[cfg(not(parallel_compiler))]
215-
let mut shard = state.active.lock();
216184
let job = match shard.remove(&self.key).unwrap() {
217185
QueryResult::Started(job) => job,
218186
QueryResult::Poisoned => panic!(),
@@ -336,10 +304,7 @@ where
336304
Qcx: QueryContext,
337305
{
338306
let state = query.query_state(qcx);
339-
#[cfg(parallel_compiler)]
340307
let mut state_lock = state.active.get_shard_by_value(&key).lock();
341-
#[cfg(not(parallel_compiler))]
342-
let mut state_lock = state.active.lock();
343308

344309
// For the parallel compiler we need to check both the query cache and query state structures
345310
// while holding the state lock to ensure that 1) the query has not yet completed and 2) the

0 commit comments

Comments
 (0)