Skip to content

Remove conditional use of Sharded from query state #114894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Remove conditional use of Sharded from query state #114894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b74cb78
Remove conditional use of `Sharded` from query state
Zoxc Aug 16, 2023
f458b11
Optimize `lock_shards`
Zoxc Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions compiler/rustc_data_structures/src/sharded.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ use crate::fx::{FxHashMap, FxHasher};
#[cfg(parallel_compiler)]
use crate::sync::{is_dyn_thread_safe, CacheAligned};
use crate::sync::{Lock, LockGuard};
#[cfg(parallel_compiler)]
use itertools::Either;
use std::borrow::Borrow;
use std::collections::hash_map::RawEntryMut;
use std::hash::{Hash, Hasher};
use std::iter;
use std::mem;

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

pub fn lock_shards(&self) -> Vec<LockGuard<'_, T>> {
#[inline]
pub fn lock_shards(&self) -> impl Iterator<Item = LockGuard<'_, T>> {
match self {
Self::Single(single) => vec![single.lock()],
#[cfg(not(parallel_compiler))]
Self::Single(single) => iter::once(single.lock()),
#[cfg(parallel_compiler)]
Self::Single(single) => Either::Left(iter::once(single.lock())),
#[cfg(parallel_compiler)]
Self::Shards(shards) => shards.iter().map(|shard| shard.0.lock()).collect(),
Self::Shards(shards) => Either::Right(shards.iter().map(|shard| shard.0.lock())),
}
}

pub fn try_lock_shards(&self) -> Option<Vec<LockGuard<'_, T>>> {
#[inline]
pub fn try_lock_shards(&self) -> impl Iterator<Item = Option<LockGuard<'_, T>>> {
match self {
Self::Single(single) => Some(vec![single.try_lock()?]),
#[cfg(not(parallel_compiler))]
Self::Single(single) => iter::once(single.try_lock()),
#[cfg(parallel_compiler)]
Self::Single(single) => Either::Left(iter::once(single.try_lock())),
#[cfg(parallel_compiler)]
Self::Shards(shards) => shards.iter().map(|shard| shard.0.try_lock()).collect(),
Self::Shards(shards) => Either::Right(shards.iter().map(|shard| shard.0.try_lock())),
}
}
}
Expand All @@ -101,7 +112,7 @@ pub type ShardedHashMap<K, V> = Sharded<FxHashMap<K, V>>;

impl<K: Eq, V> ShardedHashMap<K, V> {
pub fn len(&self) -> usize {
self.lock_shards().iter().map(|shard| shard.len()).sum()
self.lock_shards().map(|shard| shard.len()).sum()
}
}

Expand Down
39 changes: 20 additions & 19 deletions compiler/rustc_middle/src/ty/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1296,25 +1296,26 @@ macro_rules! sty_debug_print {
};
$(let mut $variant = total;)*

let shards = tcx.interners.type_.lock_shards();
let types = shards.iter().flat_map(|shard| shard.keys());
for &InternedInSet(t) in types {
let variant = match t.internee {
ty::Bool | ty::Char | ty::Int(..) | ty::Uint(..) |
ty::Float(..) | ty::Str | ty::Never => continue,
ty::Error(_) => /* unimportant */ continue,
$(ty::$variant(..) => &mut $variant,)*
};
let lt = t.flags.intersects(ty::TypeFlags::HAS_RE_INFER);
let ty = t.flags.intersects(ty::TypeFlags::HAS_TY_INFER);
let ct = t.flags.intersects(ty::TypeFlags::HAS_CT_INFER);

variant.total += 1;
total.total += 1;
if lt { total.lt_infer += 1; variant.lt_infer += 1 }
if ty { total.ty_infer += 1; variant.ty_infer += 1 }
if ct { total.ct_infer += 1; variant.ct_infer += 1 }
if lt && ty && ct { total.all_infer += 1; variant.all_infer += 1 }
for shard in tcx.interners.type_.lock_shards() {
let types = shard.keys();
for &InternedInSet(t) in types {
let variant = match t.internee {
ty::Bool | ty::Char | ty::Int(..) | ty::Uint(..) |
ty::Float(..) | ty::Str | ty::Never => continue,
ty::Error(_) => /* unimportant */ continue,
$(ty::$variant(..) => &mut $variant,)*
};
let lt = t.flags.intersects(ty::TypeFlags::HAS_RE_INFER);
let ty = t.flags.intersects(ty::TypeFlags::HAS_TY_INFER);
let ct = t.flags.intersects(ty::TypeFlags::HAS_CT_INFER);

variant.total += 1;
total.total += 1;
if lt { total.lt_infer += 1; variant.lt_infer += 1 }
if ty { total.ty_infer += 1; variant.ty_infer += 1 }
if ct { total.ct_infer += 1; variant.ct_infer += 1 }
if lt && ty && ct { total.all_infer += 1; variant.all_infer += 1 }
}
}
writeln!(fmt, "Ty interner total ty lt ct all")?;
$(writeln!(fmt, " {:18}: {uses:6} {usespc:4.1}%, \
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_query_system/src/query/caches.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ where
}

fn iter(&self, f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex)) {
let shards = self.cache.lock_shards();
for shard in shards.iter() {
for shard in self.cache.lock_shards() {
for (k, v) in shard.iter() {
f(k, &v.0, v.1);
}
Expand Down Expand Up @@ -160,8 +159,7 @@ where
}

fn iter(&self, f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex)) {
let shards = self.cache.lock_shards();
for shard in shards.iter() {
for shard in self.cache.lock_shards() {
for (k, v) in shard.iter_enumerated() {
if let Some(v) = v {
f(&k, &v.0, v.1);
Expand Down
51 changes: 8 additions & 43 deletions compiler/rustc_query_system/src/query/plumbing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobI
use crate::query::SerializedDepNodeIndex;
use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
use crate::HandleCycleError;
#[cfg(parallel_compiler)]
use rustc_data_structures::cold_path;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sharded::Sharded;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::sync::Lock;
#[cfg(parallel_compiler)]
use rustc_data_structures::{cold_path, sharded::Sharded};
use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed, FatalError};
use rustc_span::{Span, DUMMY_SP};
use std::cell::Cell;
Expand All @@ -30,10 +31,7 @@ use thin_vec::ThinVec;
use super::QueryConfig;

pub struct QueryState<K, D: DepKind> {
#[cfg(parallel_compiler)]
active: Sharded<FxHashMap<K, QueryResult<D>>>,
#[cfg(not(parallel_compiler))]
active: Lock<FxHashMap<K, QueryResult<D>>>,
}

/// Indicates the state of a query for a given key in a query map.
Expand All @@ -52,15 +50,7 @@ where
D: DepKind,
{
pub fn all_inactive(&self) -> bool {
#[cfg(parallel_compiler)]
{
let shards = self.active.lock_shards();
shards.iter().all(|shard| shard.is_empty())
}
#[cfg(not(parallel_compiler))]
{
self.active.lock().is_empty()
}
self.active.lock_shards().all(|shard| shard.is_empty())
}

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

#[cfg(parallel_compiler)]
{
// We use try_lock_shards here since we are called from the
// deadlock handler, and this shouldn't be locked.
let shards = self.active.try_lock_shards()?;
for shard in shards.iter() {
for (k, v) in shard.iter() {
if let QueryResult::Started(ref job) = *v {
active.push((*k, job.clone()));
}
}
}
}
#[cfg(not(parallel_compiler))]
{
// We use try_lock here since we are called from the
// deadlock handler, and this shouldn't be locked.
// (FIXME: Is this relevant for non-parallel compilers? It doesn't
// really hurt much.)
for (k, v) in self.active.try_lock()?.iter() {
// We use try_lock_shards here since we are called from the
// deadlock handler, and this shouldn't be locked.
for shard in self.active.try_lock_shards() {
for (k, v) in shard?.iter() {
if let QueryResult::Started(ref job) = *v {
active.push((*k, job.clone()));
}
Expand Down Expand Up @@ -184,10 +158,7 @@ where
cache.complete(key, result, dep_node_index);

let job = {
#[cfg(parallel_compiler)]
let mut lock = state.active.get_shard_by_value(&key).lock();
#[cfg(not(parallel_compiler))]
let mut lock = state.active.lock();
match lock.remove(&key).unwrap() {
QueryResult::Started(job) => job,
QueryResult::Poisoned => panic!(),
Expand All @@ -209,10 +180,7 @@ where
// Poison the query so jobs waiting on it panic.
let state = self.state;
let job = {
#[cfg(parallel_compiler)]
let mut shard = state.active.get_shard_by_value(&self.key).lock();
#[cfg(not(parallel_compiler))]
let mut shard = state.active.lock();
let job = match shard.remove(&self.key).unwrap() {
QueryResult::Started(job) => job,
QueryResult::Poisoned => panic!(),
Expand Down Expand Up @@ -325,10 +293,7 @@ where
Qcx: QueryContext,
{
let state = query.query_state(qcx);
#[cfg(parallel_compiler)]
let mut state_lock = state.active.get_shard_by_value(&key).lock();
#[cfg(not(parallel_compiler))]
let mut state_lock = state.active.lock();

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