Skip to content

Commit

Permalink
Auto merge of #56613 - Zoxc:query-perf1, r=<try>
Browse files Browse the repository at this point in the history
Tweak query code for performance

Split from #56509

r? @michaelwoerister
  • Loading branch information
bors committed Dec 10, 2018
2 parents 9567a1c + ac56bf3 commit c89a860
Show file tree
Hide file tree
Showing 12 changed files with 185 additions and 82 deletions.
33 changes: 22 additions & 11 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ macro_rules! define_dep_nodes {
}
}

#[inline]
// FIXME: Make `is_anon`, `is_input`, `is_eval_always` and `has_params` properties
// of queries
#[inline(always)]
pub fn is_anon(&self) -> bool {
match *self {
$(
Expand All @@ -171,16 +173,20 @@ macro_rules! define_dep_nodes {
}
}

#[inline]
pub fn is_input(&self) -> bool {
#[inline(always)]
pub fn is_input_inlined(&self) -> bool {
match *self {
$(
DepKind :: $variant => { contains_input_attr!($($attr),*) }
)*
}
}

#[inline]
pub fn is_input(&self) -> bool {
self.is_input_inlined()
}

#[inline(always)]
pub fn is_eval_always(&self) -> bool {
match *self {
$(
Expand All @@ -190,8 +196,8 @@ macro_rules! define_dep_nodes {
}

#[allow(unreachable_code)]
#[inline]
pub fn has_params(&self) -> bool {
#[inline(always)]
pub fn has_params_inlined(&self) -> bool {
match *self {
$(
DepKind :: $variant => {
Expand All @@ -212,6 +218,10 @@ macro_rules! define_dep_nodes {
)*
}
}

pub fn has_params(&self) -> bool {
self.has_params_inlined()
}
}

pub enum DepConstructor<$tcx> {
Expand All @@ -230,6 +240,7 @@ macro_rules! define_dep_nodes {

impl DepNode {
#[allow(unreachable_code, non_snake_case)]
#[inline(always)]
pub fn new<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
dep: DepConstructor<'gcx>)
-> DepNode
Expand Down Expand Up @@ -299,7 +310,7 @@ macro_rules! define_dep_nodes {
/// Construct a DepNode from the given DepKind and DefPathHash. This
/// method will assert that the given DepKind actually requires a
/// single DefId/DefPathHash parameter.
#[inline]
#[inline(always)]
pub fn from_def_path_hash(kind: DepKind,
def_path_hash: DefPathHash)
-> DepNode {
Expand All @@ -313,9 +324,9 @@ macro_rules! define_dep_nodes {
/// Create a new, parameterless DepNode. This method will assert
/// that the DepNode corresponding to the given DepKind actually
/// does not require any parameters.
#[inline]
#[inline(always)]
pub fn new_no_params(kind: DepKind) -> DepNode {
assert!(!kind.has_params());
assert!(!kind.has_params_inlined());
DepNode {
kind,
hash: Fingerprint::ZERO,
Expand Down Expand Up @@ -418,14 +429,14 @@ impl fmt::Debug for DepNode {


impl DefPathHash {
#[inline]
#[inline(always)]
pub fn to_dep_node(self, kind: DepKind) -> DepNode {
DepNode::from_def_path_hash(kind, self)
}
}

impl DefId {
#[inline]
#[inline(always)]
pub fn to_dep_node(self, tcx: TyCtxt<'_, '_, '_>, kind: DepKind) -> DepNode {
DepNode::from_def_path_hash(kind, tcx.def_path_hash(self))
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ impl Forest {
self.dep_graph.read(DepNode::new_no_params(DepKind::Krate));
&self.krate
}

pub fn untracked_krate<'hir>(&'hir self) -> &'hir Crate {
&self.krate
}
}

/// Represents a mapping from Node IDs to AST elements and their parent
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ impl<'a> StableHashingContext<'a> {
// The `krate` here is only used for mapping BodyIds to Bodies.
// Don't use it for anything else or you'll run the risk of
// leaking data out of the tracking system.
#[inline]
pub fn new(sess: &'a Session,
krate: &'a hir::Crate,
definitions: &'a Definitions,
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@
#![feature(slice_sort_by_cached_key)]
#![feature(specialization)]
#![feature(unboxed_closures)]
#![feature(thread_local)]
#![feature(trace_macros)]
#![feature(trusted_len)]
#![feature(vec_remove_item)]
#![feature(step_trait)]
#![feature(stmt_expr_attributes)]
#![feature(integer_atomics)]
#![feature(test)]
#![feature(in_band_lifetimes)]
Expand Down
20 changes: 17 additions & 3 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ pub struct Session {
/// Used by -Z profile-queries in util::common
pub profile_channel: Lock<Option<mpsc::Sender<ProfileQueriesMsg>>>,

/// Used by -Z self-profile
pub self_profiling_active: bool,

/// Used by -Z self-profile
pub self_profiling: Lock<SelfProfiler>,

Expand Down Expand Up @@ -825,10 +828,17 @@ impl Session {
}
}

#[inline(never)]
#[cold]
fn profiler_active<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {
let mut profiler = self.self_profiling.borrow_mut();
f(&mut profiler);
}

#[inline(always)]
pub fn profiler<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {
if self.opts.debugging_opts.self_profile || self.opts.debugging_opts.profile_json {
let mut profiler = self.self_profiling.borrow_mut();
f(&mut profiler);
if unlikely!(self.self_profiling_active) {
self.profiler_active(f)
}
}

Expand Down Expand Up @@ -1138,6 +1148,9 @@ pub fn build_session_(
CguReuseTracker::new_disabled()
};

let self_profiling_active = sopts.debugging_opts.self_profile ||
sopts.debugging_opts.profile_json;

let sess = Session {
target: target_cfg,
host,
Expand Down Expand Up @@ -1168,6 +1181,7 @@ pub fn build_session_(
imported_macro_spans: OneThread::new(RefCell::new(FxHashMap::default())),
incr_comp_session: OneThread::new(RefCell::new(IncrCompSession::NotInitialized)),
cgu_reuse_tracker,
self_profiling_active,
self_profiling: Lock::new(SelfProfiler::new()),
profile_channel: Lock::new(None),
perf_stats: PerfStats {
Expand Down
21 changes: 16 additions & 5 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,8 +1336,9 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.cstore.crate_data_as_rc_any(cnum)
}

#[inline(always)]
pub fn create_stable_hashing_context(self) -> StableHashingContext<'a> {
let krate = self.dep_graph.with_ignore(|| self.hir().krate());
let krate = self.gcx.hir_map.forest.untracked_krate();

StableHashingContext::new(self.sess,
krate,
Expand Down Expand Up @@ -1925,23 +1926,33 @@ pub mod tls {

/// A thread local variable which stores a pointer to the current ImplicitCtxt
#[cfg(not(parallel_queries))]
thread_local!(static TLV: Cell<usize> = Cell::new(0));
// Accessing `thread_local` in another crate is bugged, so we have
// two accessors `set_raw_tlv` and `get_tlv` which do not have an
// inline attribute to prevent that
#[thread_local]
static TLV: Cell<usize> = Cell::new(0);

/// This is used to set the pointer to the current ImplicitCtxt.
#[cfg(not(parallel_queries))]
fn set_raw_tlv(value: usize) {
TLV.set(value)
}

/// Sets TLV to `value` during the call to `f`.
/// It is restored to its previous value after.
/// This is used to set the pointer to the new ImplicitCtxt.
#[cfg(not(parallel_queries))]
fn set_tlv<F: FnOnce() -> R, R>(value: usize, f: F) -> R {
let old = get_tlv();
let _reset = OnDrop(move || TLV.with(|tlv| tlv.set(old)));
TLV.with(|tlv| tlv.set(value));
let _reset = OnDrop(move || set_raw_tlv(old));
set_raw_tlv(value);
f()
}

/// This is used to get the pointer to the current ImplicitCtxt.
#[cfg(not(parallel_queries))]
fn get_tlv() -> usize {
TLV.with(|tlv| tlv.get())
TLV.get()
}

/// This is a callback from libsyntax as it cannot access the implicit state
Expand Down
69 changes: 41 additions & 28 deletions src/librustc/ty/query/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ use syntax_pos::Span;
use ty::tls;
use ty::query::Query;
use ty::query::plumbing::CycleError;
#[cfg(not(parallel_queries))]
use ty::query::{
plumbing::TryGetJob,
config::QueryDescription,
};
use ty::context::TyCtxt;
use errors::Diagnostic;
use std::process;
Expand Down Expand Up @@ -83,44 +88,52 @@ impl<'tcx> QueryJob<'tcx> {
///
/// For single threaded rustc there's no concurrent jobs running, so if we are waiting for any
/// query that means that there is a query cycle, thus this always running a cycle error.
pub(super) fn await<'lcx>(
#[cfg(not(parallel_queries))]
#[inline(never)]
#[cold]
pub(super) fn await<'lcx, 'a, D: QueryDescription<'tcx>>(
&self,
tcx: TyCtxt<'_, 'tcx, 'lcx>,
span: Span,
) -> Result<(), CycleError<'tcx>> {
#[cfg(not(parallel_queries))]
{
self.find_cycle_in_stack(tcx, span)
}
) -> TryGetJob<'a, 'tcx, D> {
TryGetJob::JobCompleted(Err(Box::new(self.find_cycle_in_stack(tcx, span))))
}

#[cfg(parallel_queries)]
{
tls::with_related_context(tcx, move |icx| {
let mut waiter = Lrc::new(QueryWaiter {
query: icx.query.clone(),
span,
cycle: Lock::new(None),
condvar: Condvar::new(),
});
self.latch.await(&waiter);
// FIXME: Get rid of this lock. We have ownership of the QueryWaiter
// although another thread may still have a Lrc reference so we cannot
// use Lrc::get_mut
let mut cycle = waiter.cycle.lock();
match cycle.take() {
None => Ok(()),
Some(cycle) => Err(cycle)
}
})
}
/// Awaits for the query job to complete.
///
/// For single threaded rustc there's no concurrent jobs running, so if we are waiting for any
/// query that means that there is a query cycle, thus this always running a cycle error.
#[cfg(parallel_queries)]
pub(super) fn await<'lcx>(
&self,
tcx: TyCtxt<'_, 'tcx, 'lcx>,
span: Span,
) -> Result<(), Box<CycleError<'tcx>>> {
tls::with_related_context(tcx, move |icx| {
let mut waiter = Lrc::new(QueryWaiter {
query: icx.query.clone(),
span,
cycle: Lock::new(None),
condvar: Condvar::new(),
});
self.latch.await(&waiter);
// FIXME: Get rid of this lock. We have ownership of the QueryWaiter
// although another thread may still have a Lrc reference so we cannot
// use Lrc::get_mut
let mut cycle = waiter.cycle.lock();
match cycle.take() {
None => Ok(()),
Some(cycle) => Err(Box::new(cycle))
}
})
}

#[cfg(not(parallel_queries))]
fn find_cycle_in_stack<'lcx>(
&self,
tcx: TyCtxt<'_, 'tcx, 'lcx>,
span: Span,
) -> Result<(), CycleError<'tcx>> {
) -> CycleError<'tcx> {
// Get the current executing query (waiter) and find the waitee amongst its parents
let mut current_job = tls::with_related_context(tcx, |icx| icx.query.clone());
let mut cycle = Vec::new();
Expand All @@ -140,7 +153,7 @@ impl<'tcx> QueryJob<'tcx> {
let usage = job.parent.as_ref().map(|parent| {
(job.info.span, parent.info.query.clone())
});
return Err(CycleError { usage, cycle });
return CycleError { usage, cycle };
}

current_job = job.parent.clone();
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,21 +705,21 @@ impl<'a, 'tcx, 'lcx> TyCtxt<'a, 'tcx, 'lcx> {
self,
span: Span,
key: DefId,
) -> Result<&'tcx [Ty<'tcx>], DiagnosticBuilder<'a>> {
) -> Result<&'tcx [Ty<'tcx>], Box<DiagnosticBuilder<'a>>> {
self.try_get_query::<queries::adt_sized_constraint<'_>>(span, key)
}
pub fn try_needs_drop_raw(
self,
span: Span,
key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<bool, DiagnosticBuilder<'a>> {
) -> Result<bool, Box<DiagnosticBuilder<'a>>> {
self.try_get_query::<queries::needs_drop_raw<'_>>(span, key)
}
pub fn try_optimized_mir(
self,
span: Span,
key: DefId,
) -> Result<&'tcx mir::Mir<'tcx>, DiagnosticBuilder<'a>> {
) -> Result<&'tcx mir::Mir<'tcx>, Box<DiagnosticBuilder<'a>>> {
self.try_get_query::<queries::optimized_mir<'_>>(span, key)
}
}
Expand Down
Loading

0 comments on commit c89a860

Please sign in to comment.