Create QuerySideEffects and use it for diagnostics

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -5,23 +5,20 @@ 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::unlikely;
-use rustc_errors::Diagnostic;
 use rustc_index::vec::IndexVec;
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
 
-use parking_lot::{Condvar, Mutex};
+use parking_lot::Mutex;
 use smallvec::{smallvec, SmallVec};
 use std::collections::hash_map::Entry;
 use std::hash::Hash;
 use std::marker::PhantomData;
-use std::mem;
 use std::sync::atomic::Ordering::Relaxed;
 
 use super::query::DepGraphQuery;
 use super::serialized::{GraphEncoder, SerializedDepGraph, SerializedDepNodeIndex};
 use super::{DepContext, DepKind, DepNode, HasDepContext, WorkProductId};
-use crate::query::QueryContext;
+use crate::query::{QueryContext, QuerySideEffects};
 
 #[cfg(debug_assertions)]
 use {super::debug::EdgeFilter, std::env};
@@ -87,11 +84,7 @@ struct DepGraphData<K: DepKind> {
 
     colors: DepNodeColorMap,
 
-    /// A set of loaded diagnostics that is in the progress of being emitted.
-    emitting_diagnostics: Mutex<FxHashSet<DepNodeIndex>>,
-
-    /// Used to wait for diagnostics to be emitted.
-    emitting_diagnostics_cond_var: Condvar,
+    processed_side_effects: Mutex<FxHashSet<DepNodeIndex>>,
 
     /// When we load, there may be `.o` files, cached MIR, or other such
     /// things available to us. If we find that they are not dirty, we
@@ -144,8 +137,7 @@ impl<K: DepKind> DepGraph<K> {
                 previous_work_products: prev_work_products,
                 dep_node_debug: Default::default(),
                 current,
-                emitting_diagnostics: Default::default(),
-                emitting_diagnostics_cond_var: Condvar::new(),
+                processed_side_effects: Default::default(),
                 previous: prev_graph,
                 colors: DepNodeColorMap::new(prev_graph_node_count),
             })),
@@ -691,7 +683,7 @@ impl<K: DepKind> DepGraph<K> {
 
         // FIXME: Store the fact that a node has diagnostics in a bit in the dep graph somewhere
         // Maybe store a list on disk and encode this fact in the DepNodeState
-        let diagnostics = tcx.load_diagnostics(prev_dep_node_index);
+        let side_effects = tcx.load_side_effects(prev_dep_node_index);
 
         #[cfg(not(parallel_compiler))]
         debug_assert!(
@@ -701,8 +693,8 @@ impl<K: DepKind> DepGraph<K> {
             dep_node
         );
 
-        if unlikely!(!diagnostics.is_empty()) {
-            self.emit_diagnostics(tcx, data, dep_node_index, prev_dep_node_index, diagnostics);
+        if unlikely!(!side_effects.is_empty()) {
+            self.emit_side_effects(tcx, data, dep_node_index, side_effects);
         }
 
         // ... and finally storing a "Green" entry in the color map.
@@ -717,54 +709,27 @@ impl<K: DepKind> DepGraph<K> {
     /// This may be called concurrently on multiple threads for the same dep node.
     #[cold]
     #[inline(never)]
-    fn emit_diagnostics<Ctxt: QueryContext<DepKind = K>>(
+    fn emit_side_effects<Ctxt: QueryContext<DepKind = K>>(
         &self,
         tcx: Ctxt,
         data: &DepGraphData<K>,
         dep_node_index: DepNodeIndex,
-        prev_dep_node_index: SerializedDepNodeIndex,
-        diagnostics: Vec<Diagnostic>,
+        side_effects: QuerySideEffects,
     ) {
-        let mut emitting = data.emitting_diagnostics.lock();
+        let mut processed = data.processed_side_effects.lock();
 
-        if data.colors.get(prev_dep_node_index) == Some(DepNodeColor::Green(dep_node_index)) {
-            // The node is already green so diagnostics must have been emitted already
-            return;
-        }
-
-        if emitting.insert(dep_node_index) {
+        if processed.insert(dep_node_index) {
             // We were the first to insert the node in the set so this thread
-            // must emit the diagnostics and signal other potentially waiting
-            // threads after.
-            mem::drop(emitting);
+            // must process side effects
 
             // Promote the previous diagnostics to the current session.
-            tcx.store_diagnostics(dep_node_index, diagnostics.clone().into());
+            tcx.store_side_effects(dep_node_index, side_effects.clone());
 
             let handle = tcx.dep_context().sess().diagnostic();
 
-            for diagnostic in diagnostics {
+            for diagnostic in side_effects.diagnostics {
                 handle.emit_diagnostic(&diagnostic);
             }
-
-            // Mark the node as green now that diagnostics are emitted
-            data.colors.insert(prev_dep_node_index, DepNodeColor::Green(dep_node_index));
-
-            // Remove the node from the set
-            data.emitting_diagnostics.lock().remove(&dep_node_index);
-
-            // Wake up waiters
-            data.emitting_diagnostics_cond_var.notify_all();
-        } else {
-            // We must wait for the other thread to finish emitting the diagnostic
-
-            loop {
-                data.emitting_diagnostics_cond_var.wait(&mut emitting);
-                if data.colors.get(prev_dep_node_index) == Some(DepNodeColor::Green(dep_node_index))
-                {
-                    break;
-                }
-            }
         }
     }
 

compiler/rustc_query_system/src/query/mod.rs

@@ -62,6 +62,31 @@ impl QueryStackFrame {
     }
 }
 
+/// Tracks 'side effects' for a particular query.
+/// This struct is saved to disk along with the query result,
+/// and loaded from disk if we mark the query as green.
+/// This allows us to 'replay' changes to global state
+/// that would otherwise only occur if we actually
+/// executed the query method.
+#[derive(Debug, Clone, Default, Encodable, Decodable)]
+pub struct QuerySideEffects {
+    /// Stores any diagnostics emitted during query execution.
+    /// These diagnostics will be re-emitted if we mark
+    /// the query as green.
+    pub diagnostics: ThinVec<Diagnostic>,
+}
+
+impl QuerySideEffects {
+    pub fn is_empty(&self) -> bool {
+        let QuerySideEffects { diagnostics } = self;
+        diagnostics.is_empty()
+    }
+    pub fn append(&mut self, other: QuerySideEffects) {
+        let QuerySideEffects { diagnostics } = self;
+        diagnostics.extend(other.diagnostics);
+    }
+}
+
 pub trait QueryContext: HasDepContext {
     /// Get the query information from the TLS context.
     fn current_query_job(&self) -> Option<QueryJobId<Self::DepKind>>;
@@ -74,17 +99,17 @@ pub trait QueryContext: HasDepContext {
     /// Try to force a dep node to execute and see if it's green.
     fn try_force_from_dep_node(&self, dep_node: &DepNode<Self::DepKind>) -> bool;
 
-    /// Load diagnostics associated to the node in the previous session.
-    fn load_diagnostics(&self, prev_dep_node_index: SerializedDepNodeIndex) -> Vec<Diagnostic>;
+    /// Load side effects associated to the node in the previous session.
+    fn load_side_effects(&self, prev_dep_node_index: SerializedDepNodeIndex) -> QuerySideEffects;
 
     /// Register diagnostics for the given node, for use in next session.
-    fn store_diagnostics(&self, dep_node_index: DepNodeIndex, diagnostics: ThinVec<Diagnostic>);
+    fn store_side_effects(&self, dep_node_index: DepNodeIndex, side_effects: QuerySideEffects);
 
     /// Register diagnostics for the given node, for use in next session.
-    fn store_diagnostics_for_anon_node(
+    fn store_side_effects_for_anon_node(
         &self,
         dep_node_index: DepNodeIndex,
-        diagnostics: ThinVec<Diagnostic>,
+        side_effects: QuerySideEffects,
     );
 
     /// Executes a job by changing the `ImplicitCtxt` to point to the

compiler/rustc_query_system/src/query/plumbing.rs

@@ -9,7 +9,7 @@ use crate::query::config::{QueryDescription, QueryVtable, QueryVtableExt};
 use crate::query::job::{
     report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryShardJobId,
 };
-use crate::query::{QueryContext, QueryMap, QueryStackFrame};
+use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
 
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxHashMap, FxHasher};
@@ -479,8 +479,10 @@ where
 
         dep_graph.read_index(dep_node_index);
 
-        if unlikely!(!diagnostics.is_empty()) {
-            tcx.store_diagnostics_for_anon_node(dep_node_index, diagnostics);
+        let side_effects = QuerySideEffects { diagnostics };
+
+        if unlikely!(!side_effects.is_empty()) {
+            tcx.store_side_effects_for_anon_node(dep_node_index, side_effects);
         }
 
         return job.complete(result, dep_node_index);
@@ -677,8 +679,10 @@ where
 
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
-    if unlikely!(!diagnostics.is_empty()) && dep_node.kind != DepKind::NULL {
-        tcx.store_diagnostics(dep_node_index, diagnostics);
+    let side_effects = QuerySideEffects { diagnostics };
+
+    if unlikely!(!side_effects.is_empty()) && dep_node.kind != DepKind::NULL {
+        tcx.store_side_effects(dep_node_index, side_effects);
     }
 
     let result = job.complete(result, dep_node_index);