Auto merge of #56613 - Zoxc:query-perf1, r=<try>

bors · bors · commit c89a860cdedf · 2018-12-10T12:59:13.000Z
Tweak query code for performance Split from #56509 r? @michaelwoerister
diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs
@@ -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 {
                     $(
@@ -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 {
                     $(
@@ -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 => {
@@ -212,6 +218,10 @@ macro_rules! define_dep_nodes {
                     )*
                 }
             }
+
+            pub fn has_params(&self) -> bool {
+                self.has_params_inlined()
+            }
         }
 
         pub enum DepConstructor<$tcx> {
@@ -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
@@ -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 {
@@ -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,
@@ -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))
     }
diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs
@@ -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
diff --git a/src/librustc/ich/hcx.rs b/src/librustc/ich/hcx.rs
@@ -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,
diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs
@@ -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)]
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
@@ -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>,
 
@@ -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)
         }
     }
 
@@ -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,
@@ -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 {
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -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,
@@ -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
diff --git a/src/librustc/ty/query/job.rs b/src/librustc/ty/query/job.rs
@@ -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;
@@ -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();
@@ -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();
diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs
@@ -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)
     }
 }
diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs
diff --git a/src/librustc_data_structures/fingerprint.rs b/src/librustc_data_structures/fingerprint.rs
diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs
diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs

Original file line number	Diff line number	Diff line change
`@@ -159,6 +159,10 @@ impl Forest {`
`159`	`159`	`self.dep_graph.read(DepNode::new_no_params(DepKind::Krate));`
`160`	`160`	`&self.krate`
`161`	`161`	`}`
	`162`	`+`
	`163`	`+ pub fn untracked_krate<'hir>(&'hir self) -> &'hir Crate {`
	`164`	`+ &self.krate`
	`165`	`+ }`
`162`	`166`	`}`
`163`	`167`
`164`	`168`	`/// Represents a mapping from Node IDs to AST elements and their parent`