Tweak query code for performance

Zoxc · Zoxc · commit cf3e67e3e715 · 2018-12-04T19:28:53.000+01:00
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,7 +240,8 @@ macro_rules! define_dep_nodes {
 
         impl DepNode {
             #[allow(unreachable_code, non_snake_case)]
-            pub fn new<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
+            #[inline(always)]
+            pub fn new_inlined<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
                                        dep: DepConstructor<'gcx>)
                                        -> DepNode
                 where 'gcx: 'a + 'tcx,
@@ -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/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs
@@ -239,6 +239,7 @@ impl DepGraph {
         arg: A,
         no_tcx: bool,
         task: fn(C, A) -> R,
+        // FIXME: Take OpenTask as a parameter instead
         create_task: fn(DepNode) -> OpenTask,
         finish_task_and_alloc_depnode: fn(&Lock<CurrentDepGraph>,
                                           DepNode,
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,6 +60,7 @@
 #![feature(slice_sort_by_cached_key)]
 #![feature(specialization)]
 #![feature(unboxed_closures)]
+#![feature(thread_local)]
 #![feature(trace_macros)]
 #![feature(trusted_len)]
 #![feature(vec_remove_item)]
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
@@ -825,13 +825,22 @@ impl Session {
         }
     }
 
-    pub fn profiler<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {
+    #[inline(never)]
+    #[cold]
+    fn profiler_active<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {
         if self.opts.debugging_opts.self_profile {
             let mut profiler = self.self_profiling.borrow_mut();
             f(&mut profiler);
         }
     }
 
+    #[inline(always)]
+    pub fn profiler<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {
+        if unsafe { std::intrinsics::unlikely(self.opts.debugging_opts.self_profile) } {
+            self.profiler_active(f)
+        }
+    }
+
     pub fn print_profiler_results(&self) {
         let mut profiler = self.self_profiling.borrow_mut();
         profiler.print_results(&self.opts);
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -1331,8 +1331,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.gcx.hir.krate());
+        let krate = self.gcx.hir.forest.untracked_krate();
 
         StableHashingContext::new(self.sess,
                                   krate,
@@ -1349,7 +1350,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         // We cannot use the query versions of crates() and crate_hash(), since
         // those would need the DepNodes that we are allocating here.
         for cnum in self.cstore.crates_untracked() {
-            let dep_node = DepNode::new(self, DepConstructor::CrateMetadata(cnum));
+            let dep_node = DepNode::new_inlined(self, DepConstructor::CrateMetadata(cnum));
             let crate_hash = self.cstore.crate_hash_untracked(cnum);
             self.dep_graph.with_task(dep_node,
                                      self,
@@ -1926,23 +1927,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,41 +88,49 @@ 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)
-        }
-
-        #[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);
+    ) -> TryGetJob<'a, 'tcx, D> {
+        TryGetJob::JobCompleted(Err(Box::new(self.find_cycle_in_stack(tcx, span))))
+    }
 
-                match Lrc::get_mut(&mut waiter).unwrap().cycle.get_mut().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);
+
+            match Lrc::get_mut(&mut waiter).unwrap().cycle.get_mut().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();
@@ -137,7 +150,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_mir/monomorphize/partitioning.rs b/src/librustc_mir/monomorphize/partitioning.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`
Original file line number	Diff line number	Diff line change
`@@ -825,13 +825,22 @@ impl Session {`
`825`	`825`	`}`
`826`	`826`	`}`
`827`	`827`
`828`		`- pub fn profiler<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {`
	`828`	`+ #[inline(never)]`
	`829`	`+ #[cold]`
	`830`	`+ fn profiler_active<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {`
`829`	`831`	`if self.opts.debugging_opts.self_profile {`
`830`	`832`	`let mut profiler = self.self_profiling.borrow_mut();`
`831`	`833`	`f(&mut profiler);`
`832`	`834`	`}`
`833`	`835`	`}`
`834`	`836`
	`837`	`+ #[inline(always)]`
	`838`	`+ pub fn profiler<F: FnOnce(&mut SelfProfiler) -> ()>(&self, f: F) {`
	`839`	`+ if unsafe { std::intrinsics::unlikely(self.opts.debugging_opts.self_profile) } {`
	`840`	`+ self.profiler_active(f)`
	`841`	`+ }`
	`842`	`+ }`
	`843`	`+`
`835`	`844`	`pub fn print_profiler_results(&self) {`
`836`	`845`	`let mut profiler = self.self_profiling.borrow_mut();`
`837`	`846`	`profiler.print_results(&self.opts);`