Do not adjust ImplicitCtxt twice for each query.

Author: cjgillot

compiler/rustc_incremental/src/assert_dep_graph.rs

@@ -55,7 +55,7 @@ use std::io::{BufWriter, Write};
 pub fn assert_dep_graph(tcx: TyCtxt<'_>) {
     tcx.dep_graph.with_ignore(|| {
         if tcx.sess.opts.debugging_opts.dump_dep_graph {
-            tcx.dep_graph.with_query(dump_graph);
+            tcx.dep_graph.with_debug(dump_graph);
         }
 
         if !tcx.sess.opts.debugging_opts.query_dep_graph {
@@ -207,7 +207,7 @@ fn check_paths<'tcx>(tcx: TyCtxt<'tcx>, if_this_changed: &Sources, then_this_wou
         }
         return;
     }
-    tcx.dep_graph.with_query(|query| {
+    tcx.dep_graph.with_debug(|query| {
         for &(_, source_def_id, ref source_dep_node) in if_this_changed {
             let dependents = query.transitive_predecessors(source_dep_node);
             for &(target_span, ref target_pass, _, ref target_dep_node) in then_this_would_need {

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -1,13 +1,17 @@
-use parking_lot::Mutex;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::profiling::{EventId, QueryInvocationId, SelfProfilerRef};
 use rustc_data_structures::sharded::{self, Sharded};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::steal::Steal;
 use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc, Ordering};
+use rustc_data_structures::thin_vec::ThinVec;
+use rustc_data_structures::unlikely;
+use rustc_errors::Diagnostic;
 use rustc_index::vec::IndexVec;
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
+
+use parking_lot::Mutex;
 use smallvec::{smallvec, SmallVec};
 use std::collections::hash_map::Entry;
 use std::fmt::Debug;
@@ -19,7 +23,7 @@ use super::query::DepGraphQuery;
 use super::serialized::{GraphEncoder, SerializedDepGraph, SerializedDepNodeIndex};
 use super::{DepContext, DepKind, DepNode, HasDepContext, WorkProductId};
 use crate::ich::StableHashingContext;
-use crate::query::{QueryContext, QuerySideEffects};
+use crate::query::{QueryContext, QueryJobId, QuerySideEffects};
 
 #[cfg(debug_assertions)]
 use {super::debug::EdgeFilter, std::env};
@@ -151,7 +155,7 @@ impl DepGraph {
         self.data.is_some()
     }
 
-    pub fn with_query(&self, f: impl Fn(&DepGraphQuery)) {
+    pub fn with_debug(&self, f: impl Fn(&DepGraphQuery)) {
         if let Some(data) = &self.data {
             data.current.encoder.borrow().with_query(f)
         }
@@ -209,7 +213,13 @@ impl DepGraph {
         hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
     ) -> (R, DepNodeIndex) {
         if self.is_fully_enabled() {
-            self.with_task_impl(key, cx, arg, task, hash_result)
+            self.with_task_impl(
+                key,
+                cx,
+                arg,
+                |arg, task_deps| crate::tls::with_deps(task_deps, || task(cx, arg)),
+                hash_result,
+            )
         } else {
             // Incremental compilation is turned off. We just execute the task
             // without tracking. We still provide a dep-node index that uniquely
@@ -219,12 +229,35 @@ impl DepGraph {
         }
     }
 
+    pub(crate) fn with_query<Ctxt: QueryContext, A: Debug, R>(
+        &self,
+        key: DepNode,
+        cx: Ctxt,
+        arg: A,
+        token: QueryJobId,
+        diagnostics: Option<&Lock<ThinVec<Diagnostic>>>,
+        task: fn(Ctxt::DepContext, A) -> R,
+        hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
+    ) -> (R, DepNodeIndex) {
+        self.with_task_impl(
+            key,
+            cx,
+            arg,
+            |arg, task_deps| {
+                crate::tls::start_query(token, diagnostics, task_deps, || {
+                    task(*cx.dep_context(), arg)
+                })
+            },
+            hash_result,
+        )
+    }
+
     fn with_task_impl<Ctxt: HasDepContext, A: Debug, R>(
         &self,
         key: DepNode,
         cx: Ctxt,
         arg: A,
-        task: fn(Ctxt, A) -> R,
+        invoke: impl FnOnce(A, Option<&Lock<TaskDeps>>) -> R,
         hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
     ) -> (R, DepNodeIndex) {
         // This function is only called when the graph is enabled.
@@ -255,7 +288,7 @@ impl DepGraph {
                 phantom_data: PhantomData,
             }))
         };
-        let result = crate::tls::with_deps(task_deps.as_ref(), || task(cx, arg));
+        let result = invoke(arg, task_deps.as_ref());
         let edges = task_deps.map_or_else(|| smallvec![], |lock| lock.into_inner().reads);
 
         let dcx = cx.dep_context();
@@ -306,57 +339,88 @@ impl DepGraph {
     {
         debug_assert!(!cx.is_eval_always(dep_kind));
 
-        if let Some(ref data) = self.data {
-            let task_deps = Lock::new(TaskDeps::default());
-            let result = crate::tls::with_deps(Some(&task_deps), op);
-            let task_deps = task_deps.into_inner();
-            let task_deps = task_deps.reads;
-
-            let dep_node_index = match task_deps.len() {
-                0 => {
-                    // Because the dep-node id of anon nodes is computed from the sets of its
-                    // dependencies we already know what the ID of this dependency-less node is
-                    // going to be (i.e. equal to the precomputed
-                    // `SINGLETON_DEPENDENCYLESS_ANON_NODE`). As a consequence we can skip creating
-                    // a `StableHasher` and sending the node through interning.
-                    DepNodeIndex::SINGLETON_DEPENDENCYLESS_ANON_NODE
-                }
-                1 => {
-                    // When there is only one dependency, don't bother creating a node.
-                    task_deps[0]
-                }
-                _ => {
-                    // The dep node indices are hashed here instead of hashing the dep nodes of the
-                    // dependencies. These indices may refer to different nodes per session, but this isn't
-                    // a problem here because we that ensure the final dep node hash is per session only by
-                    // combining it with the per session random number `anon_id_seed`. This hash only need
-                    // to map the dependencies to a single value on a per session basis.
-                    let mut hasher = StableHasher::new();
-                    task_deps.hash(&mut hasher);
-
-                    let target_dep_node = DepNode {
-                        kind: dep_kind,
-                        // Fingerprint::combine() is faster than sending Fingerprint
-                        // through the StableHasher (at least as long as StableHasher
-                        // is so slow).
-                        hash: data.current.anon_id_seed.combine(hasher.finish()).into(),
-                    };
-
-                    data.current.intern_new_node(
-                        cx.profiler(),
-                        target_dep_node,
-                        task_deps,
-                        Fingerprint::ZERO,
-                    )
-                }
-            };
-
-            (result, dep_node_index)
+        if self.is_fully_enabled() {
+            self.with_anon_task_impl(*cx.dep_context(), dep_kind, |task_deps| {
+                crate::tls::with_deps(task_deps, op)
+            })
         } else {
             (op(), self.next_virtual_depnode_index())
         }
     }
 
+    /// Executes something within an "anonymous" task, that is, a task the
+    /// `DepNode` of which is determined by the list of inputs it read from.
+    pub(crate) fn with_anon_query<Ctxt: QueryContext, A, R>(
+        &self,
+        dep_kind: DepKind,
+        cx: Ctxt,
+        arg: A,
+        token: QueryJobId,
+        diagnostics: Option<&Lock<ThinVec<Diagnostic>>>,
+        task: fn(Ctxt::DepContext, A) -> R,
+    ) -> (R, DepNodeIndex) {
+        debug_assert!(!cx.dep_context().is_eval_always(dep_kind));
+
+        self.with_anon_task_impl(*cx.dep_context(), dep_kind, |task_deps| {
+            crate::tls::start_query(token, diagnostics, task_deps, || task(*cx.dep_context(), arg))
+        })
+    }
+
+    fn with_anon_task_impl<Ctxt: DepContext, R>(
+        &self,
+        cx: Ctxt,
+        dep_kind: DepKind,
+        invoke: impl FnOnce(Option<&Lock<TaskDeps>>) -> R,
+    ) -> (R, DepNodeIndex) {
+        debug_assert!(!cx.dep_context().is_eval_always(dep_kind));
+
+        let data = self.data.as_ref().unwrap();
+        let task_deps = Lock::new(TaskDeps::default());
+        let result = invoke(Some(&task_deps));
+        let task_deps = task_deps.into_inner().reads;
+
+        let dep_node_index = match task_deps.len() {
+            0 => {
+                // Because the dep-node id of anon nodes is computed from the sets of its
+                // dependencies we already know what the ID of this dependency-less node is
+                // going to be (i.e. equal to the precomputed
+                // `SINGLETON_DEPENDENCYLESS_ANON_NODE`). As a consequence we can skip creating
+                // a `StableHasher` and sending the node through interning.
+                DepNodeIndex::SINGLETON_DEPENDENCYLESS_ANON_NODE
+            }
+            1 => {
+                // When there is only one dependency, don't bother creating a node.
+                task_deps[0]
+            }
+            _ => {
+                // The dep node indices are hashed here instead of hashing the dep nodes of the
+                // dependencies. These indices may refer to different nodes per session, but this isn't
+                // a problem here because we that ensure the final dep node hash is per session only by
+                // combining it with the per session random number `anon_id_seed`. This hash only need
+                // to map the dependencies to a single value on a per session basis.
+                let mut hasher = StableHasher::new();
+                task_deps.hash(&mut hasher);
+
+                let target_dep_node = DepNode {
+                    kind: dep_kind,
+                    // Fingerprint::combine() is faster than sending Fingerprint
+                    // through the StableHasher (at least as long as StableHasher
+                    // is so slow).
+                    hash: data.current.anon_id_seed.combine(hasher.finish()).into(),
+                };
+
+                data.current.intern_new_node(
+                    cx.profiler(),
+                    target_dep_node,
+                    task_deps,
+                    Fingerprint::ZERO,
+                )
+            }
+        };
+
+        (result, dep_node_index)
+    }
+
     #[inline]
     pub fn read_index(&self, dep_node_index: DepNodeIndex) {
         if let Some(ref data) = self.data {

compiler/rustc_query_system/src/query/plumbing.rs

@@ -429,7 +429,7 @@ where
     if !dep_graph.is_fully_enabled() {
         let prof_timer = tcx.dep_context().profiler().query_provider();
         let result =
-            crate::tls::start_query(job_id, None, || query.compute(*tcx.dep_context(), key));
+            crate::tls::start_query(job_id, None, None, || query.compute(*tcx.dep_context(), key));
         let dep_node_index = dep_graph.next_virtual_depnode_index();
         prof_timer.finish_with_query_invocation_id(dep_node_index.into());
         return (result, dep_node_index);
@@ -442,7 +442,7 @@ where
 
         // The diagnostics for this query will be promoted to the current session during
         // `try_mark_green()`, so we can ignore them here.
-        if let Some(ret) = crate::tls::start_query(job_id, None, || {
+        if let Some(ret) = crate::tls::start_query(job_id, None, None, || {
             try_load_from_disk_and_cache_in_memory(tcx, &key, &dep_node, query)
         }) {
             return ret;
@@ -452,18 +452,29 @@ where
     let prof_timer = tcx.dep_context().profiler().query_provider();
     let diagnostics = Lock::new(ThinVec::new());
 
-    let (result, dep_node_index) = crate::tls::start_query(job_id, Some(&diagnostics), || {
-        if query.anon {
-            return dep_graph.with_anon_task(*tcx.dep_context(), query.dep_kind, || {
-                query.compute(*tcx.dep_context(), key)
-            });
-        }
-
+    let (result, dep_node_index) = if query.anon {
+        dep_graph.with_anon_query(
+            query.dep_kind,
+            tcx,
+            key,
+            job_id,
+            Some(&diagnostics),
+            query.compute,
+        )
+    } else {
         // `to_dep_node` is expensive for some `DepKind`s.
         let dep_node = dep_node_opt.unwrap_or_else(|| query.to_dep_node(*tcx.dep_context(), &key));
 
-        dep_graph.with_task(dep_node, *tcx.dep_context(), key, query.compute, query.hash_result)
-    });
+        dep_graph.with_query(
+            dep_node,
+            tcx,
+            key,
+            job_id,
+            Some(&diagnostics),
+            query.compute,
+            query.hash_result,
+        )
+    };
 
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 

compiler/rustc_query_system/src/tls.rs

@@ -140,15 +140,12 @@ pub fn current_query_job() -> Option<QueryJobId> {
 pub fn start_query<R>(
     token: QueryJobId,
     diagnostics: Option<&Lock<ThinVec<Diagnostic>>>,
+    task_deps: Option<&Lock<TaskDeps>>,
     compute: impl FnOnce() -> R,
 ) -> R {
-    with_context_opt(move |current_icx| {
-        let task_deps = current_icx.and_then(|icx| icx.task_deps);
+    // Update the `ImplicitCtxt` to point to our new query job.
+    let new_icx = ImplicitCtxt { query: Some(token), diagnostics, task_deps };
 
-        // Update the `ImplicitCtxt` to point to our new query job.
-        let new_icx = ImplicitCtxt { query: Some(token), diagnostics, task_deps };
-
-        // Use the `ImplicitCtxt` while we execute the query.
-        enter_context(&new_icx, |_| rustc_data_structures::stack::ensure_sufficient_stack(compute))
-    })
+    // Use the `ImplicitCtxt` while we execute the query.
+    enter_context(&new_icx, |_| rustc_data_structures::stack::ensure_sufficient_stack(compute))
 }