Auto merge of #57114 - Zoxc:query-perf11, r=michaelwoerister

Clean up and optimize OpenTask / read_index

r? @michaelwoerister

5 files changed, 131 insertions, 172 deletions

diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs
index 961638151a2..71cacfe3706 100644
--- a/src/librustc/dep_graph/graph.rs
+++ b/src/librustc/dep_graph/graph.rs
@@ -122,12 +122,7 @@ impl DepGraph {
         if let Some(..) = self.data {
             ty::tls::with_context_opt(|icx| {
                 let icx = if let Some(icx) = icx { icx } else { return };
-                match *icx.task {
-                    OpenTask::Ignore => {
-                        // ignored
-                    }
-                    _ => panic!("expected an ignore context")
-                }
+                assert!(icx.task_deps.is_none(), "expected no task dependency tracking");
             })
         }
     }
@@ -137,7 +132,7 @@ impl DepGraph {
     {
         ty::tls::with_context(|icx| {
             let icx = ty::tls::ImplicitCtxt {
-                task: &OpenTask::Ignore,
+                task_deps: None,
                 ..icx.clone()
             };
 
@@ -184,12 +179,15 @@ impl DepGraph {
               R: HashStable<StableHashingContext<'gcx>>,
     {
         self.with_task_impl(key, cx, arg, false, task,
-            |key| OpenTask::Regular(Lock::new(RegularOpenTask {
-                node: key,
+            |_key| Some(TaskDeps {
+                #[cfg(debug_assertions)]
+                node: Some(_key),
                 reads: SmallVec::new(),
                 read_set: Default::default(),
-            })),
-            |data, key, fingerprint, task| data.borrow_mut().complete_task(key, task, fingerprint))
+            }),
+            |data, key, fingerprint, task| {
+                data.borrow_mut().complete_task(key, task.unwrap(), fingerprint)
+            })
     }
 
     /// Creates a new dep-graph input with value `input`
@@ -206,7 +204,7 @@ impl DepGraph {
         }
 
         self.with_task_impl(key, cx, input, true, identity_fn,
-            |_| OpenTask::Ignore,
+            |_| None,
             |data, key, fingerprint, _| {
                 data.borrow_mut().alloc_node(key, SmallVec::new(), fingerprint)
             })
@@ -219,18 +217,18 @@ impl DepGraph {
         arg: A,
         no_tcx: bool,
         task: fn(C, A) -> R,
-        create_task: fn(DepNode) -> OpenTask,
+        create_task: fn(DepNode) -> Option<TaskDeps>,
         finish_task_and_alloc_depnode: fn(&Lock<CurrentDepGraph>,
                                           DepNode,
                                           Fingerprint,
-                                          OpenTask) -> DepNodeIndex
+                                          Option<TaskDeps>) -> DepNodeIndex
     ) -> (R, DepNodeIndex)
     where
         C: DepGraphSafe + StableHashingContextProvider<'gcx>,
         R: HashStable<StableHashingContext<'gcx>>,
     {
         if let Some(ref data) = self.data {
-            let open_task = create_task(key);
+            let task_deps = create_task(key).map(|deps| Lock::new(deps));
 
             // In incremental mode, hash the result of the task. We don't
             // do anything with the hash yet, but we are computing it
@@ -248,7 +246,7 @@ impl DepGraph {
             } else {
                 ty::tls::with_context(|icx| {
                     let icx = ty::tls::ImplicitCtxt {
-                        task: &open_task,
+                        task_deps: task_deps.as_ref(),
                         ..icx.clone()
                     };
 
@@ -271,7 +269,7 @@ impl DepGraph {
                 &data.current,
                 key,
                 current_fingerprint,
-                open_task
+                task_deps.map(|lock| lock.into_inner()),
             );
 
             // Determine the color of the new DepNode.
@@ -304,15 +302,17 @@ impl DepGraph {
         where OP: FnOnce() -> R
     {
         if let Some(ref data) = self.data {
-            let (result, open_task) = ty::tls::with_context(|icx| {
-                let task = OpenTask::Anon(Lock::new(AnonOpenTask {
+            let (result, task_deps) = ty::tls::with_context(|icx| {
+                let task_deps = Lock::new(TaskDeps {
+                    #[cfg(debug_assertions)]
+                    node: None,
                     reads: SmallVec::new(),
                     read_set: Default::default(),
-                }));
+                });
 
                 let r = {
                     let icx = ty::tls::ImplicitCtxt {
-                        task: &task,
+                        task_deps: Some(&task_deps),
                         ..icx.clone()
                     };
 
@@ -321,11 +321,11 @@ impl DepGraph {
                     })
                 };
 
-                (r, task)
+                (r, task_deps.into_inner())
             });
             let dep_node_index = data.current
                                      .borrow_mut()
-                                     .pop_anon_task(dep_kind, open_task);
+                                     .complete_anon_task(dep_kind, task_deps);
             (result, dep_node_index)
         } else {
             (op(), DepNodeIndex::INVALID)
@@ -344,18 +344,23 @@ impl DepGraph {
               R: HashStable<StableHashingContext<'gcx>>,
     {
         self.with_task_impl(key, cx, arg, false, task,
-            |key| OpenTask::EvalAlways { node: key },
-            |data, key, fingerprint, task| {
-                data.borrow_mut().complete_eval_always_task(key, task, fingerprint)
+            |_| None,
+            |data, key, fingerprint, _| {
+                let mut current = data.borrow_mut();
+                let krate_idx = current.node_to_node_index[
+                    &DepNode::new_no_params(DepKind::Krate)
+                ];
+                current.alloc_node(key, smallvec![krate_idx], fingerprint)
             })
     }
 
     #[inline]
     pub fn read(&self, v: DepNode) {
         if let Some(ref data) = self.data {
-            let mut current = data.current.borrow_mut();
+            let current = data.current.borrow_mut();
             if let Some(&dep_node_index) = current.node_to_node_index.get(&v) {
-                current.read_index(dep_node_index);
+                std::mem::drop(current);
+                data.read_index(dep_node_index);
             } else {
                 bug!("DepKind {:?} should be pre-allocated but isn't.", v.kind)
             }
@@ -365,7 +370,7 @@ impl DepGraph {
     #[inline]
     pub fn read_index(&self, dep_node_index: DepNodeIndex) {
         if let Some(ref data) = self.data {
-            data.current.borrow_mut().read_index(dep_node_index);
+            data.read_index(dep_node_index);
         }
     }
 
@@ -446,10 +451,15 @@ impl DepGraph {
             .cloned()
     }
 
-    pub fn edge_deduplication_data(&self) -> (u64, u64) {
-        let current_dep_graph = self.data.as_ref().unwrap().current.borrow();
+    pub fn edge_deduplication_data(&self) -> Option<(u64, u64)> {
+        if cfg!(debug_assertions) {
+            let current_dep_graph = self.data.as_ref().unwrap().current.borrow();
 
-        (current_dep_graph.total_read_count, current_dep_graph.total_duplicate_read_count)
+            Some((current_dep_graph.total_read_count,
+                  current_dep_graph.total_duplicate_read_count))
+        } else {
+            None
+        }
     }
 
     pub fn serialize(&self) -> SerializedDepGraph {
@@ -827,6 +837,7 @@ struct DepNodeData {
 pub(super) struct CurrentDepGraph {
     data: IndexVec<DepNodeIndex, DepNodeData>,
     node_to_node_index: FxHashMap<DepNode, DepNodeIndex>,
+    #[allow(dead_code)]
     forbidden_edge: Option<EdgeFilter>,
 
     // Anonymous DepNodes are nodes the ID of which we compute from the list of
@@ -890,134 +901,60 @@ impl CurrentDepGraph {
 
     fn complete_task(
         &mut self,
-        key: DepNode,
-        task: OpenTask,
+        node: DepNode,
+        task_deps: TaskDeps,
         fingerprint: Fingerprint
     ) -> DepNodeIndex {
-        if let OpenTask::Regular(task) = task {
-            let RegularOpenTask {
-                node,
-                read_set: _,
-                reads
-            } = task.into_inner();
-            assert_eq!(node, key);
-
-            // If this is an input node, we expect that it either has no
-            // dependencies, or that it just depends on DepKind::CrateMetadata
-            // or DepKind::Krate. This happens for some "thin wrapper queries"
-            // like `crate_disambiguator` which sometimes have zero deps (for
-            // when called for LOCAL_CRATE) or they depend on a CrateMetadata
-            // node.
-            if cfg!(debug_assertions) {
-                if node.kind.is_input() && reads.len() > 0 &&
-                   // FIXME(mw): Special case for DefSpan until Spans are handled
-                   //            better in general.
-                   node.kind != DepKind::DefSpan &&
-                    reads.iter().any(|&i| {
-                        !(self.data[i].node.kind == DepKind::CrateMetadata ||
-                          self.data[i].node.kind == DepKind::Krate)
-                    })
-                {
-                    bug!("Input node {:?} with unexpected reads: {:?}",
-                        node,
-                        reads.iter().map(|&i| self.data[i].node).collect::<Vec<_>>())
-                }
+        // If this is an input node, we expect that it either has no
+        // dependencies, or that it just depends on DepKind::CrateMetadata
+        // or DepKind::Krate. This happens for some "thin wrapper queries"
+        // like `crate_disambiguator` which sometimes have zero deps (for
+        // when called for LOCAL_CRATE) or they depend on a CrateMetadata
+        // node.
+        if cfg!(debug_assertions) {
+            if node.kind.is_input() && task_deps.reads.len() > 0 &&
+                // FIXME(mw): Special case for DefSpan until Spans are handled
+                //            better in general.
+                node.kind != DepKind::DefSpan &&
+                task_deps.reads.iter().any(|&i| {
+                    !(self.data[i].node.kind == DepKind::CrateMetadata ||
+                        self.data[i].node.kind == DepKind::Krate)
+                })
+            {
+                bug!("Input node {:?} with unexpected reads: {:?}",
+                    node,
+                    task_deps.reads.iter().map(|&i| self.data[i].node).collect::<Vec<_>>())
             }
-
-            self.alloc_node(node, reads, fingerprint)
-        } else {
-            bug!("complete_task() - Expected regular task to be popped")
         }
-    }
-
-    fn pop_anon_task(&mut self, kind: DepKind, task: OpenTask) -> DepNodeIndex {
-        if let OpenTask::Anon(task) = task {
-            let AnonOpenTask {
-                read_set: _,
-                reads
-            } = task.into_inner();
-            debug_assert!(!kind.is_input());
-
-            let mut fingerprint = self.anon_id_seed;
-            let mut hasher = StableHasher::new();
 
-            for &read in reads.iter() {
-                let read_dep_node = self.data[read].node;
+        self.alloc_node(node, task_deps.reads, fingerprint)
+    }
 
-                ::std::mem::discriminant(&read_dep_node.kind).hash(&mut hasher);
+    fn complete_anon_task(&mut self, kind: DepKind, task_deps: TaskDeps) -> DepNodeIndex {
+        debug_assert!(!kind.is_input());
 
-                // Fingerprint::combine() is faster than sending Fingerprint
-                // through the StableHasher (at least as long as StableHasher
-                // is so slow).
-                fingerprint = fingerprint.combine(read_dep_node.hash);
-            }
+        let mut fingerprint = self.anon_id_seed;
+        let mut hasher = StableHasher::new();
 
-            fingerprint = fingerprint.combine(hasher.finish());
+        for &read in task_deps.reads.iter() {
+            let read_dep_node = self.data[read].node;
 
-            let target_dep_node = DepNode {
-                kind,
-                hash: fingerprint,
-            };
+            ::std::mem::discriminant(&read_dep_node.kind).hash(&mut hasher);
 
-            self.intern_node(target_dep_node, reads, Fingerprint::ZERO).0
-        } else {
-            bug!("pop_anon_task() - Expected anonymous task to be popped")
+            // Fingerprint::combine() is faster than sending Fingerprint
+            // through the StableHasher (at least as long as StableHasher
+            // is so slow).
+            fingerprint = fingerprint.combine(read_dep_node.hash);
         }
-    }
 
-    fn complete_eval_always_task(
-        &mut self,
-        key: DepNode,
-        task: OpenTask,
-        fingerprint: Fingerprint
-    ) -> DepNodeIndex {
-        if let OpenTask::EvalAlways {
-            node,
-        } = task {
-            debug_assert_eq!(node, key);
-            let krate_idx = self.node_to_node_index[&DepNode::new_no_params(DepKind::Krate)];
-            self.alloc_node(node, smallvec![krate_idx], fingerprint)
-        } else {
-            bug!("complete_eval_always_task() - Expected eval always task to be popped");
-        }
-    }
+        fingerprint = fingerprint.combine(hasher.finish());
 
-    fn read_index(&mut self, source: DepNodeIndex) {
-        ty::tls::with_context_opt(|icx| {
-            let icx = if let Some(icx) = icx { icx } else { return };
-            match *icx.task {
-                OpenTask::Regular(ref task) => {
-                    let mut task = task.lock();
-                    self.total_read_count += 1;
-                    if task.read_set.insert(source) {
-                        task.reads.push(source);
-
-                        if cfg!(debug_assertions) {
-                            if let Some(ref forbidden_edge) = self.forbidden_edge {
-                                let target = &task.node;
-                                let source = self.data[source].node;
-                                if forbidden_edge.test(&source, &target) {
-                                    bug!("forbidden edge {:?} -> {:?} created",
-                                        source,
-                                        target)
-                                }
-                            }
-                        }
-                    } else {
-                        self.total_duplicate_read_count += 1;
-                    }
-                }
-                OpenTask::Anon(ref task) => {
-                    let mut task = task.lock();
-                    if task.read_set.insert(source) {
-                        task.reads.push(source);
-                    }
-                }
-                OpenTask::Ignore | OpenTask::EvalAlways { .. } => {
-                    // ignore
-                }
-            }
-        })
+        let target_dep_node = DepNode {
+            kind,
+            hash: fingerprint,
+        };
+
+        self.intern_node(target_dep_node, task_deps.reads, Fingerprint::ZERO).0
     }
 
     fn alloc_node(
@@ -1054,26 +991,47 @@ impl CurrentDepGraph {
     }
 }
 
-pub struct RegularOpenTask {
-    node: DepNode,
-    reads: SmallVec<[DepNodeIndex; 8]>,
-    read_set: FxHashSet<DepNodeIndex>,
+impl DepGraphData {
+    fn read_index(&self, source: DepNodeIndex) {
+        ty::tls::with_context_opt(|icx| {
+            let icx = if let Some(icx) = icx { icx } else {  return };
+            if let Some(task_deps) = icx.task_deps {
+                let mut task_deps = task_deps.lock();
+                if cfg!(debug_assertions) {
+                    self.current.lock().total_read_count += 1;
+                }
+                if task_deps.read_set.insert(source) {
+                    task_deps.reads.push(source);
+
+                    #[cfg(debug_assertions)]
+                    {
+                        if let Some(target) = task_deps.node {
+                            let graph = self.current.lock();
+                            if let Some(ref forbidden_edge) = graph.forbidden_edge {
+                                let source = graph.data[source].node;
+                                if forbidden_edge.test(&source, &target) {
+                                    bug!("forbidden edge {:?} -> {:?} created",
+                                        source,
+                                        target)
+                                }
+                            }
+                        }
+                    }
+                } else if cfg!(debug_assertions) {
+                    self.current.lock().total_duplicate_read_count += 1;
+                }
+            }
+        })
+    }
 }
 
-pub struct AnonOpenTask {
+pub struct TaskDeps {
+    #[cfg(debug_assertions)]
+    node: Option<DepNode>,
     reads: SmallVec<[DepNodeIndex; 8]>,
     read_set: FxHashSet<DepNodeIndex>,
 }
 
-pub enum OpenTask {
-    Regular(Lock<RegularOpenTask>),
-    Anon(Lock<AnonOpenTask>),
-    Ignore,
-    EvalAlways {
-        node: DepNode,
-    },
-}
-
 // A data structure that stores Option<DepNodeColor> values as a contiguous
 // array, using one u32 per entry.
 struct DepNodeColorMap {
diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs
index d153b7435c9..022caabcbf3 100644
--- a/src/librustc/dep_graph/mod.rs
+++ b/src/librustc/dep_graph/mod.rs
@@ -10,7 +10,7 @@ pub mod cgu_reuse_tracker;
 
 pub use self::dep_tracking_map::{DepTrackingMap, DepTrackingMapConfig};
 pub use self::dep_node::{DepNode, DepKind, DepConstructor, WorkProductId, label_strs};
-pub use self::graph::{DepGraph, WorkProduct, DepNodeIndex, DepNodeColor, OpenTask};
+pub use self::graph::{DepGraph, WorkProduct, DepNodeIndex, DepNodeColor, TaskDeps};
 pub use self::graph::WorkProductFileKind;
 pub use self::prev::PreviousDepGraph;
 pub use self::query::DepGraphQuery;
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
index d4da10bcbed..c0ba4329ae0 100644
--- a/src/librustc/ty/context.rs
+++ b/src/librustc/ty/context.rs
@@ -1674,7 +1674,7 @@ impl<'gcx> GlobalCtxt<'gcx> {
                 tcx,
                 query: icx.query.clone(),
                 layout_depth: icx.layout_depth,
-                task: icx.task,
+                task_deps: icx.task_deps,
             };
             ty::tls::enter_context(&new_icx, |_| {
                 f(tcx)
@@ -1782,7 +1782,7 @@ pub mod tls {
     use errors::{Diagnostic, TRACK_DIAGNOSTICS};
     use rustc_data_structures::OnDrop;
     use rustc_data_structures::sync::{self, Lrc, Lock};
-    use dep_graph::OpenTask;
+    use dep_graph::TaskDeps;
 
     #[cfg(not(parallel_queries))]
     use std::cell::Cell;
@@ -1810,7 +1810,7 @@ pub mod tls {
 
         /// The current dep graph task. This is used to add dependencies to queries
         /// when executing them
-        pub task: &'a OpenTask,
+        pub task_deps: Option<&'a Lock<TaskDeps>>,
     }
 
     /// Sets Rayon's thread local variable which is preserved for Rayon jobs
@@ -1939,7 +1939,7 @@ pub mod tls {
                 tcx,
                 query: None,
                 layout_depth: 0,
-                task: &OpenTask::Ignore,
+                task_deps: None,
             };
             enter_context(&icx, |_| {
                 f(tcx)
@@ -1969,7 +1969,7 @@ pub mod tls {
             query: None,
             tcx,
             layout_depth: 0,
-            task: &OpenTask::Ignore,
+            task_deps: None,
         };
         enter_context(&icx, |_| f(tcx))
     }
diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs
index 19198e90a81..2d619d19b42 100644
--- a/src/librustc/ty/query/plumbing.rs
+++ b/src/librustc/ty/query/plumbing.rs
@@ -209,7 +209,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {
                 tcx: tcx.global_tcx(),
                 query: Some(self.job.clone()),
                 layout_depth: current_icx.layout_depth,
-                task: current_icx.task,
+                task_deps: current_icx.task_deps,
             };
 
             // Use the ImplicitCtxt while we execute the query
diff --git a/src/librustc_incremental/persist/save.rs b/src/librustc_incremental/persist/save.rs
index 5131b63115a..6a7553b3882 100644
--- a/src/librustc_incremental/persist/save.rs
+++ b/src/librustc_incremental/persist/save.rs
@@ -149,8 +149,6 @@ fn encode_dep_graph(tcx: TyCtxt,
 
         let total_node_count = serialized_graph.nodes.len();
         let total_edge_count = serialized_graph.edge_list_data.len();
-        let (total_edge_reads, total_duplicate_edge_reads) =
-            tcx.dep_graph.edge_deduplication_data();
 
         let mut counts: FxHashMap<_, Stat> = FxHashMap::default();
 
@@ -188,8 +186,11 @@ fn encode_dep_graph(tcx: TyCtxt,
         println!("[incremental]");
         println!("[incremental] Total Node Count: {}", total_node_count);
         println!("[incremental] Total Edge Count: {}", total_edge_count);
-        println!("[incremental] Total Edge Reads: {}", total_edge_reads);
-        println!("[incremental] Total Duplicate Edge Reads: {}", total_duplicate_edge_reads);
+        if let Some((total_edge_reads,
+                     total_duplicate_edge_reads)) = tcx.dep_graph.edge_deduplication_data() {
+            println!("[incremental] Total Edge Reads: {}", total_edge_reads);
+            println!("[incremental] Total Duplicate Edge Reads: {}", total_duplicate_edge_reads);
+        }
         println!("[incremental]");
         println!("[incremental]  {:<36}| {:<17}| {:<12}| {:<17}|",
                  "Node Kind",