Auto merge of #56509 - Zoxc:query-perf, r=<try>

bors · bors · commit da5ac1d0bbe0 · 2018-12-04T16:24:38.000Z
[WIP] Tweak query code for performance 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 {
                     $(
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/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
@@ -1926,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,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.
+    #[cfg(not(parallel_queries))]
+    #[inline(never)]
+    #[cold]
+    pub(super) fn await<'lcx, 'a, D: QueryDescription<'tcx>>(
+        &self,
+        tcx: TyCtxt<'_, 'tcx, 'lcx>,
+        span: Span,
+    ) -> TryGetJob<'a, 'tcx, D> {
+        TryGetJob::JobCompleted(Err(self.find_cycle_in_stack(tcx, span)))
+    }
+
+    /// 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<(), 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);
-
-                match Lrc::get_mut(&mut waiter).unwrap().cycle.get_mut().take() {
-                    None => Ok(()),
-                    Some(cycle) => Err(cycle)
-                }
-            })
-        }
+        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(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
@@ -153,8 +153,14 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {
             };
             mem::drop(lock);
 
-            if let Err(cycle) = job.await(tcx, span) {
-                return TryGetJob::JobCompleted(Err(cycle));
+            #[cfg(not(parallel_queries))]
+            return job.await(tcx, span);
+
+            #[cfg(parallel_queries)]
+            {
+                if let Err(cycle) = job.await(tcx, span) {
+                    return TryGetJob::JobCompleted(Err(cycle));
+                }
             }
         }
     }
@@ -245,8 +251,10 @@ pub(super) enum TryGetJob<'a, 'tcx: 'a, D: QueryDescription<'tcx> + 'a> {
 }
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
+    #[inline(never)]
+    #[cold]
     pub(super) fn report_cycle(self, CycleError { usage, cycle: stack }: CycleError<'gcx>)
-        -> DiagnosticBuilder<'a>
+        -> Box<DiagnosticBuilder<'a>>
     {
         assert!(!stack.is_empty());
 
@@ -280,7 +288,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
                               &format!("cycle used when {}", query.describe(self)));
             }
 
-            return err
+            return Box::new(err)
         })
     }
 
@@ -345,6 +353,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    #[inline(never)]
     fn try_get_with<Q: QueryDescription<'gcx>>(
         self,
         span: Span,
@@ -409,7 +418,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             return Ok(result);
         }
 
-        if !dep_node.kind.is_input() {
+        if !dep_node.kind.is_input_inlined() {
             if let Some(dep_node_index) = self.try_mark_green_and_read(&dep_node) {
                 profq_msg!(self, ProfileQueriesMsg::CacheHit);
                 self.sess.profiler(|p| p.record_query_hit(Q::CATEGORY));
@@ -585,7 +594,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
 
         // Ensuring an "input" or anonymous query makes no sense
         assert!(!dep_node.kind.is_anon());
-        assert!(!dep_node.kind.is_input());
+        assert!(!dep_node.kind.is_input_inlined());
         if self.try_mark_green_and_read(&dep_node).is_none() {
             // A None return from `try_mark_green_and_read` means that this is either
             // a new dep node or that the dep node has already been marked red.
@@ -625,7 +634,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         self,
         span: Span,
         key: Q::Key,
-    ) -> Result<Q::Value, DiagnosticBuilder<'a>> {
+    ) -> Result<Q::Value, Box<DiagnosticBuilder<'a>>> {
         match self.try_get_with::<Q>(span, key) {
             Ok(e) => Ok(e),
             Err(e) => Err(self.report_cycle(e)),
@@ -637,11 +646,20 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         span: Span,
         key: Q::Key,
     ) -> Q::Value {
-        self.try_get_query::<Q>(span, key).unwrap_or_else(|mut e| {
-            e.emit();
-            Q::handle_cycle_error(self)
+        self.try_get_query::<Q>(span, key).unwrap_or_else(|e| {
+            self.emit_error::<Q>(e)
         })
     }
+
+    #[inline(never)]
+    #[cold]
+    fn emit_error<Q: QueryDescription<'gcx>>(
+        self,
+        mut e: Box<DiagnosticBuilder<'a>>,
+    ) -> Q::Value {
+        e.emit();
+        Q::handle_cycle_error(self)
+    }
 }
 
 macro_rules! handle_cycle_error {
@@ -861,6 +879,7 @@ macro_rules! define_queries_inner {
 
         impl<'a, 'gcx, 'tcx> Deref for TyCtxtAt<'a, 'gcx, 'tcx> {
             type Target = TyCtxt<'a, 'gcx, 'tcx>;
+            #[inline(always)]
             fn deref(&self) -> &Self::Target {
                 &self.tcx
             }
@@ -869,6 +888,7 @@ macro_rules! define_queries_inner {
         impl<'a, $tcx, 'lcx> TyCtxt<'a, $tcx, 'lcx> {
             /// Return a transparent wrapper for `TyCtxt` which uses
             /// `span` as the location of queries performed through it.
+            #[inline(always)]
             pub fn at(self, span: Span) -> TyCtxtAt<'a, $tcx, 'lcx> {
                 TyCtxtAt {
                     tcx: self,
@@ -877,13 +897,15 @@ macro_rules! define_queries_inner {
             }
 
             $($(#[$attr])*
+            #[inline(always)]
             pub fn $name(self, key: $K) -> $V {
                 self.at(DUMMY_SP).$name(key)
             })*
         }
 
         impl<'a, $tcx, 'lcx> TyCtxtAt<'a, $tcx, 'lcx> {
             $($(#[$attr])*
+            #[inline(always)]
             pub fn $name(self, key: $K) -> $V {
                 self.tcx.get_query::<queries::$name<'_>>(self.span, key)
             })*

Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,9 @@ macro_rules! define_dep_nodes {`
`162`	`162`	`}`
`163`	`163`	`}`
`164`	`164`
`165`		`- #[inline]`
	`165`	+ // FIXME: Make `is_anon`, `is_input`, `is_eval_always` and `has_params` properties
	`166`	`+ // of queries`
	`167`	`+ #[inline(always)]`
`166`	`168`	`pub fn is_anon(&self) -> bool {`
`167`	`169`	`match *self {`
`168`	`170`	`$(`
`@@ -171,16 +173,20 @@ macro_rules! define_dep_nodes {`
`171`	`173`	`}`
`172`	`174`	`}`
`173`	`175`
`174`		`- #[inline]`
`175`		`- pub fn is_input(&self) -> bool {`
	`176`	`+ #[inline(always)]`
	`177`	`+ pub fn is_input_inlined(&self) -> bool {`
`176`	`178`	`match *self {`
`177`	`179`	`$(`
`178`	`180`	`DepKind :: $variant => { contains_input_attr!($($attr),*) }`
`179`	`181`	`)*`
`180`	`182`	`}`
`181`	`183`	`}`
`182`	`184`
`183`		`- #[inline]`
	`185`	`+ pub fn is_input(&self) -> bool {`
	`186`	`+ self.is_input_inlined()`
	`187`	`+ }`
	`188`	`+`
	`189`	`+ #[inline(always)]`
`184`	`190`	`pub fn is_eval_always(&self) -> bool {`
`185`	`191`	`match *self {`
`186`	`192`	`$(`
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);`