Always compute DepNode for incremental queries.

Currently we only compute it when necessary -- sometimes in
`check_if_ensure_can_skip_execution`, sometimes in
one of two places in `execute_job_incr` -- and pass around
`Option<DepNode>` to allow this.

This commit changes things so we always compute it up front for
incremental queries. The performance cost is negligible and it
simplifies things.
- `check_if_ensure_can_skip_execution` can be made simpler, returning a
  bool and eliminating the need for `EnsureCanSkip`.
- `execute_job_incr` no longer uses two slightly different methods to
  create a `DepNode` (`get_or_insert_with` vs `unwrap_or_else`).

Author: nnethercote

compiler/rustc_query_impl/src/execution.rs

@@ -300,7 +300,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
 
             // Delegate to another function to actually execute the query job.
             let (value, dep_node_index) = if INCR {
-                execute_job_incr(query, tcx, key, dep_node, id)
+                execute_job_incr(query, tcx, key, dep_node.unwrap(), id)
             } else {
                 execute_job_non_incr(query, tcx, key, id)
             };
@@ -416,27 +416,23 @@ fn execute_job_incr<'tcx, C: QueryCache>(
     query: &'tcx QueryVTable<'tcx, C>,
     tcx: TyCtxt<'tcx>,
     key: C::Key,
-    mut dep_node_opt: Option<DepNode>,
+    dep_node: DepNode,
     job_id: QueryJobId,
 ) -> (C::Value, DepNodeIndex) {
     let dep_graph_data =
         tcx.dep_graph.data().expect("should always be present in incremental mode");
 
     if !query.anon && !query.eval_always {
-        // `to_dep_node` is expensive for some `DepKind`s.
-        let dep_node =
-            dep_node_opt.get_or_insert_with(|| DepNode::construct(tcx, query.dep_kind, &key));
-
         // 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) = start_query(job_id, false, || try {
-            let (prev_index, dep_node_index) = dep_graph_data.try_mark_green(tcx, dep_node)?;
+            let (prev_index, dep_node_index) = dep_graph_data.try_mark_green(tcx, &dep_node)?;
             let value = load_from_disk_or_invoke_provider_green(
                 tcx,
                 dep_graph_data,
                 query,
                 key,
-                dep_node,
+                &dep_node,
                 prev_index,
                 dep_node_index,
             );
@@ -456,10 +452,6 @@ fn execute_job_incr<'tcx, C: QueryCache>(
             });
         }
 
-        // `to_dep_node` is expensive for some `DepKind`s.
-        let dep_node =
-            dep_node_opt.unwrap_or_else(|| DepNode::construct(tcx, query.dep_kind, &key));
-
         // Call the query provider.
         dep_graph_data.with_task(
             dep_node,
@@ -571,69 +563,56 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>(
     value
 }
 
-/// Return value struct for [`check_if_ensure_can_skip_execution`].
-struct EnsureCanSkip {
-    /// If true, the current `tcx.ensure_ok()` or `tcx.ensure_done()` query
-    /// can return early without actually trying to execute.
-    skip_execution: bool,
-    /// A dep node that was prepared while checking whether execution can be
-    /// skipped, to be reused by execution itself if _not_ skipped.
-    dep_node: Option<DepNode>,
-}
-
 /// Checks whether a `tcx.ensure_ok()` or `tcx.ensure_done()` query call can
 /// return early without actually trying to execute.
 ///
 /// This only makes sense during incremental compilation, because it relies
 /// on having the dependency graph (and in some cases a disk-cached value)
 /// from the previous incr-comp session.
 #[inline(never)]
-fn check_if_ensure_can_skip_execution<'tcx, C: QueryCache>(
+fn ensure_can_skip_execution<'tcx, C: QueryCache>(
     query: &'tcx QueryVTable<'tcx, C>,
     tcx: TyCtxt<'tcx>,
     key: C::Key,
+    dep_node: DepNode,
     ensure_mode: EnsureMode,
-) -> EnsureCanSkip {
+) -> bool {
     // Queries with `eval_always` should never skip execution.
     if query.eval_always {
-        return EnsureCanSkip { skip_execution: false, dep_node: None };
+        return false;
     }
 
     // Ensuring an anonymous query makes no sense
     assert!(!query.anon);
 
-    let dep_node = DepNode::construct(tcx, query.dep_kind, &key);
-
-    let serialized_dep_node_index = match tcx.dep_graph.try_mark_green(tcx, &dep_node) {
+    match tcx.dep_graph.try_mark_green(tcx, &dep_node) {
         None => {
             // A None return from `try_mark_green` means that this is either
             // a new dep node or that the dep node has already been marked red.
             // Either way, we can't call `dep_graph.read()` as we don't have the
             // DepNodeIndex. We must invoke the query itself. The performance cost
             // this introduces should be negligible as we'll immediately hit the
             // in-memory cache, or another query down the line will.
-            return EnsureCanSkip { skip_execution: false, dep_node: Some(dep_node) };
+            false
         }
         Some((serialized_dep_node_index, dep_node_index)) => {
             tcx.dep_graph.read_index(dep_node_index);
             tcx.prof.query_cache_hit(dep_node_index.into());
-            serialized_dep_node_index
-        }
-    };
-
-    match ensure_mode {
-        EnsureMode::Ok => {
-            // In ensure-ok mode, we can skip execution for this key if the node
-            // is green. It must have succeeded in the previous session, and
-            // therefore would succeed in the current session if executed.
-            EnsureCanSkip { skip_execution: true, dep_node: None }
-        }
-        EnsureMode::Done => {
-            // In ensure-done mode, we can only skip execution for this key if
-            // there's a disk-cached value available to load later if needed,
-            // which guarantees the query provider will never run for this key.
-            let is_loadable = (query.is_loadable_from_disk_fn)(tcx, key, serialized_dep_node_index);
-            EnsureCanSkip { skip_execution: is_loadable, dep_node: Some(dep_node) }
+            match ensure_mode {
+                // In ensure-ok mode, we can skip execution for this key if the
+                // node is green. It must have succeeded in the previous
+                // session, and therefore would succeed in the current session
+                // if executed.
+                EnsureMode::Ok => true,
+
+                // In ensure-done mode, we can only skip execution for this key
+                // if there's a disk-cached value available to load later if
+                // needed, which guarantees the query provider will never run
+                // for this key.
+                EnsureMode::Done => {
+                    (query.is_loadable_from_disk_fn)(tcx, key, serialized_dep_node_index)
+                }
+            }
         }
     }
 }
@@ -660,24 +639,20 @@ pub(super) fn execute_query_incr_inner<'tcx, C: QueryCache>(
     key: C::Key,
     mode: QueryMode,
 ) -> Option<C::Value> {
+    // There are scenarios where this `dep_node` is never used (e.g. for `anon` queries) but they
+    // are rare enough that the wasted effort doesn't materially hurt performance.
+    let dep_node = DepNode::construct(tcx, query.dep_kind, &key);
+
     // Check if query execution can be skipped, for `ensure_ok` or `ensure_done`.
-    // This might have the side-effect of creating a suitable DepNode, which
-    // we should reuse for execution instead of creating a new one.
-    let dep_node: Option<DepNode> = match mode {
-        QueryMode::Ensure { ensure_mode } => {
-            let EnsureCanSkip { skip_execution, dep_node } =
-                check_if_ensure_can_skip_execution(query, tcx, key, ensure_mode);
-            if skip_execution {
-                // Return early to skip execution.
-                return None;
-            }
-            dep_node
-        }
-        QueryMode::Get => None,
-    };
+    if let QueryMode::Ensure { ensure_mode } = mode
+        && ensure_can_skip_execution(query, tcx, key, dep_node, ensure_mode)
+    {
+        return None;
+    }
 
-    let (result, dep_node_index) =
-        ensure_sufficient_stack(|| try_execute_query::<C, true>(query, tcx, span, key, dep_node));
+    let (result, dep_node_index) = ensure_sufficient_stack(|| {
+        try_execute_query::<C, true>(query, tcx, span, key, Some(dep_node))
+    });
     if let Some(dep_node_index) = dep_node_index {
         tcx.dep_graph.read_index(dep_node_index)
     }