Don't store current-session side effects in OnDiskCache

Cargo.lock

@@ -4573,7 +4573,6 @@ dependencies = [
  "rustc_data_structures",
  "rustc_errors",
  "rustc_hir",
- "rustc_index",
  "rustc_macros",
  "rustc_middle",
  "rustc_serialize",

compiler/rustc_incremental/src/persist/save.rs

@@ -6,6 +6,7 @@ use rustc_data_structures::sync::par_join;
 use rustc_middle::dep_graph::{
     DepGraph, SerializedDepGraph, WorkProduct, WorkProductId, WorkProductMap,
 };
+use rustc_middle::query::on_disk_cache;
 use rustc_middle::ty::TyCtxt;
 use rustc_serialize::Encodable as RustcEncodable;
 use rustc_serialize::opaque::FileEncoder;
@@ -82,7 +83,7 @@ pub(crate) fn save_dep_graph(tcx: TyCtxt<'_>) {
 
                     file_format::save_in(sess, query_cache_path, "query cache", |encoder| {
                         tcx.sess.time("incr_comp_serialize_result_cache", || {
-                            on_disk_cache.serialize(tcx, encoder)
+                            on_disk_cache::OnDiskCache::serialize(tcx, encoder)
                         })
                     });
                 });

compiler/rustc_middle/src/dep_graph/graph.rs

@@ -709,7 +709,7 @@ impl DepGraphData {
             // side effect.
             std::iter::once(DepNodeIndex::FOREVER_RED_NODE).collect(),
         );
-        tcx.store_side_effect(dep_node_index, side_effect);
+        tcx.query_system.side_effects.borrow_mut().insert(dep_node_index, side_effect);
         dep_node_index
     }
 
@@ -718,7 +718,13 @@ impl DepGraphData {
     #[inline]
     fn force_side_effect<'tcx>(&self, tcx: TyCtxt<'tcx>, prev_index: SerializedDepNodeIndex) {
         with_deps(TaskDepsRef::Ignore, || {
-            let side_effect = tcx.load_side_effect(prev_index).unwrap();
+            let side_effect = tcx
+                .query_system
+                .on_disk_cache
+                .as_ref()
+                .unwrap()
+                .load_side_effect(tcx, prev_index)
+                .unwrap();
 
             // Use `send_and_color` as `promote_node_and_deps_to_current` expects all
             // green dependencies. `send_and_color` will also prevent multiple nodes
@@ -745,7 +751,7 @@ impl DepGraphData {
             }
 
             // This will just overwrite the same value for concurrent calls.
-            tcx.store_side_effect(dep_node_index, side_effect);
+            tcx.query_system.side_effects.borrow_mut().insert(dep_node_index, side_effect);
         })
     }
 
@@ -1549,23 +1555,4 @@ impl<'tcx> TyCtxt<'tcx> {
     fn is_eval_always(self, kind: DepKind) -> bool {
         self.dep_kind_vtable(kind).is_eval_always
     }
-
-    // Interactions with on_disk_cache
-    fn load_side_effect(
-        self,
-        prev_dep_node_index: SerializedDepNodeIndex,
-    ) -> Option<QuerySideEffect> {
-        self.query_system
-            .on_disk_cache
-            .as_ref()
-            .and_then(|c| c.load_side_effect(self, prev_dep_node_index))
-    }
-
-    #[inline(never)]
-    #[cold]
-    fn store_side_effect(self, dep_node_index: DepNodeIndex, side_effect: QuerySideEffect) {
-        if let Some(c) = self.query_system.on_disk_cache.as_ref() {
-            c.store_side_effect(dep_node_index, side_effect)
-        }
-    }
 }

compiler/rustc_middle/src/dep_graph/serialized.rs

@@ -72,6 +72,15 @@ rustc_index::newtype_index! {
     pub struct SerializedDepNodeIndex {}
 }
 
+impl SerializedDepNodeIndex {
+    /// Converts a current-session dep node index to a "serialized" index,
+    /// for the purpose of serializing data to be loaded by future sessions.
+    #[inline(always)]
+    pub fn from_curr_for_serialization(index: DepNodeIndex) -> Self {
+        SerializedDepNodeIndex::from_u32(index.as_u32())
+    }
+}
+
 const DEP_NODE_SIZE: usize = size_of::<SerializedDepNodeIndex>();
 /// Amount of padding we need to add to the edge list data so that we can retrieve every
 /// SerializedDepNodeIndex with a fixed-size read then mask.

compiler/rustc_middle/src/hooks/mod.rs

@@ -9,7 +9,7 @@ use rustc_span::def_id::{CrateNum, LocalDefId};
 use rustc_span::{ExpnHash, ExpnId};
 
 use crate::mir;
-use crate::query::on_disk_cache::{CacheEncoder, EncodedDepNodeIndex};
+use crate::query::on_disk_cache::CacheEncoder;
 use crate::ty::{Ty, TyCtxt};
 
 macro_rules! declare_hooks {
@@ -111,10 +111,8 @@ declare_hooks! {
     /// Creates the MIR for a given `DefId`, including unreachable code.
     hook build_mir_inner_impl(def: LocalDefId) -> mir::Body<'tcx>;
 
-    hook encode_query_values(
-        encoder: &mut CacheEncoder<'_, 'tcx>,
-        query_result_index: &mut EncodedDepNodeIndex
-    ) -> ();
+    /// Serializes all eligible query return values into the on-disk cache.
+    hook encode_query_values(encoder: &mut CacheEncoder<'_, 'tcx>) -> ();
 }
 
 #[cold]

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -2,14 +2,14 @@ use std::collections::hash_map::Entry;
 use std::sync::Arc;
 use std::{fmt, mem};
 
-use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
+use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
 use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::sync::{HashMapExt, Lock, RwLock};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LOCAL_CRATE, LocalDefId, StableCrateId};
 use rustc_hir::definitions::DefPathHash;
-use rustc_index::{Idx, IndexVec};
+use rustc_index::IndexVec;
 use rustc_macros::{Decodable, Encodable};
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder, IntEncodedWithFixedSize, MemDecoder};
 use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
@@ -53,22 +53,18 @@ pub struct OnDiskCache {
     // The complete cache data in serialized form.
     serialized_data: RwLock<Option<Mmap>>,
 
-    // Collects all `QuerySideEffect` created during the current compilation
-    // session.
-    current_side_effects: Lock<FxIndexMap<DepNodeIndex, QuerySideEffect>>,
-
     file_index_to_stable_id: FxHashMap<SourceFileIndex, EncodedSourceFileId>,
 
     // Caches that are populated lazily during decoding.
     file_index_to_file: Lock<FxHashMap<SourceFileIndex, Arc<SourceFile>>>,
 
-    // A map from dep-node to the position of the cached query result in
-    // `serialized_data`.
-    query_result_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
+    /// For query dep nodes that have a disk-cached return value, maps the node
+    /// index to the position of its serialized value in `serialized_data`.
+    query_values_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
 
-    // A map from dep-node to the position of any associated `QuerySideEffect` in
-    // `serialized_data`.
-    prev_side_effects_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
+    /// For `DepKind::SideEffect` dep nodes, maps the node index to the position
+    /// of its serialized [`QuerySideEffect`] in `serialized_data`.
+    side_effects_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
 
     alloc_decoding_state: AllocDecodingState,
 
@@ -101,8 +97,8 @@ pub struct OnDiskCache {
 #[derive(Encodable, Decodable)]
 struct Footer {
     file_index_to_stable_id: FxHashMap<SourceFileIndex, EncodedSourceFileId>,
-    query_result_index: EncodedDepNodeIndex,
-    side_effects_index: EncodedDepNodeIndex,
+    query_values_index: Vec<(SerializedDepNodeIndex, AbsoluteBytePos)>,
+    side_effects_index: Vec<(SerializedDepNodeIndex, AbsoluteBytePos)>,
     // The location of all allocations.
     // Most uses only need values up to u32::MAX, but benchmarking indicates that we can use a u64
     // without measurable overhead. This permits larger const allocations without ICEing.
@@ -114,8 +110,6 @@ struct Footer {
     foreign_expn_data: UnhashMap<ExpnHash, u32>,
 }
 
-pub type EncodedDepNodeIndex = Vec<(SerializedDepNodeIndex, AbsoluteBytePos)>;
-
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, Encodable, Decodable)]
 struct SourceFileIndex(u32);
 
@@ -174,9 +168,8 @@ impl OnDiskCache {
             serialized_data: RwLock::new(Some(data)),
             file_index_to_stable_id: footer.file_index_to_stable_id,
             file_index_to_file: Default::default(),
-            current_side_effects: Default::default(),
-            query_result_index: footer.query_result_index.into_iter().collect(),
-            prev_side_effects_index: footer.side_effects_index.into_iter().collect(),
+            query_values_index: footer.query_values_index.into_iter().collect(),
+            side_effects_index: footer.side_effects_index.into_iter().collect(),
             alloc_decoding_state: AllocDecodingState::new(footer.interpret_alloc_index),
             syntax_contexts: footer.syntax_contexts,
             expn_data: footer.expn_data,
@@ -190,9 +183,8 @@ impl OnDiskCache {
             serialized_data: RwLock::new(None),
             file_index_to_stable_id: Default::default(),
             file_index_to_file: Default::default(),
-            current_side_effects: Default::default(),
-            query_result_index: Default::default(),
-            prev_side_effects_index: Default::default(),
+            query_values_index: Default::default(),
+            side_effects_index: Default::default(),
             alloc_decoding_state: AllocDecodingState::new(Vec::new()),
             syntax_contexts: FxHashMap::default(),
             expn_data: UnhashMap::default(),
@@ -207,7 +199,9 @@ impl OnDiskCache {
         *self.serialized_data.write() = None;
     }
 
-    pub fn serialize(&self, tcx: TyCtxt<'_>, encoder: FileEncoder) -> FileEncodeResult {
+    /// Serialize the current-session data that will be loaded by [`OnDiskCache`]
+    /// in a subsequent incremental compilation session.
+    pub fn serialize(tcx: TyCtxt<'_>, encoder: FileEncoder) -> FileEncodeResult {
         // Serializing the `DepGraph` should not modify it.
         tcx.dep_graph.with_ignore(|| {
             // Allocate `SourceFileIndex`es.
@@ -241,30 +235,19 @@ impl OnDiskCache {
                 file_to_file_index,
                 hygiene_context: &hygiene_encode_context,
                 symbol_index_table: Default::default(),
+                query_values_index: Default::default(),
+                side_effects_index: Default::default(),
             };
 
-            // Encode query results.
-            let mut query_result_index = EncodedDepNodeIndex::new();
-
+            // Encode query return values.
             tcx.sess.time("encode_query_values", || {
-                let enc = &mut encoder;
-                let qri = &mut query_result_index;
-                tcx.encode_query_values(enc, qri);
+                tcx.encode_query_values(&mut encoder);
             });
 
             // Encode side effects.
-            let side_effects_index: EncodedDepNodeIndex = self
-                .current_side_effects
-                .borrow()
-                .iter()
-                .map(|(dep_node_index, side_effect)| {
-                    let pos = AbsoluteBytePos::new(encoder.position());
-                    let dep_node_index = SerializedDepNodeIndex::new(dep_node_index.index());
-                    encoder.encode_tagged(dep_node_index, side_effect);
-
-                    (dep_node_index, pos)
-                })
-                .collect();
+            for (&dep_node_index, side_effect) in tcx.query_system.side_effects.borrow().iter() {
+                encoder.encode_side_effect(dep_node_index, side_effect);
+            }
 
             let interpret_alloc_index = {
                 let mut interpret_alloc_index = Vec::new();
@@ -315,11 +298,13 @@ impl OnDiskCache {
 
             // Encode the file footer.
             let footer_pos = encoder.position() as u64;
+            let query_values_index = mem::take(&mut encoder.query_values_index);
+            let side_effects_index = mem::take(&mut encoder.side_effects_index);
             encoder.encode_tagged(
                 TAG_FILE_FOOTER,
                 &Footer {
                     file_index_to_stable_id,
-                    query_result_index,
+                    query_values_index,
                     side_effects_index,
                     interpret_alloc_index,
                     syntax_contexts,
@@ -340,45 +325,33 @@ impl OnDiskCache {
     }
 
     /// Loads a `QuerySideEffect` created during the previous compilation session.
-    pub fn load_side_effect(
+    pub(crate) fn load_side_effect(
         &self,
         tcx: TyCtxt<'_>,
         dep_node_index: SerializedDepNodeIndex,
     ) -> Option<QuerySideEffect> {
         let side_effect: Option<QuerySideEffect> =
-            self.load_indexed(tcx, dep_node_index, &self.prev_side_effects_index);
+            self.load_indexed(tcx, dep_node_index, &self.side_effects_index);
         side_effect
     }
 
-    /// Stores a `QuerySideEffect` emitted during the current compilation session.
-    /// Anything stored like this will be available via `load_side_effect` in
-    /// the next compilation session.
-    pub fn store_side_effect(&self, dep_node_index: DepNodeIndex, side_effect: QuerySideEffect) {
-        let mut current_side_effects = self.current_side_effects.borrow_mut();
-        let prev = current_side_effects.insert(dep_node_index, side_effect);
-        debug_assert!(prev.is_none());
-    }
-
-    /// Return whether the cached query result can be decoded.
+    /// Returns true if there is a disk-cached query return value for the given node.
     #[inline]
     pub fn loadable_from_disk(&self, dep_node_index: SerializedDepNodeIndex) -> bool {
-        self.query_result_index.contains_key(&dep_node_index)
+        self.query_values_index.contains_key(&dep_node_index)
         // with_decoder is infallible, so we can stop here
     }
 
-    /// Returns the cached query result if there is something in the cache for
-    /// the given `SerializedDepNodeIndex`; otherwise returns `None`.
-    pub fn try_load_query_result<'tcx, T>(
+    /// Returns the disk-cached query return value for the given node, if there is one.
+    pub fn try_load_query_value<'tcx, T>(
         &self,
         tcx: TyCtxt<'tcx>,
         dep_node_index: SerializedDepNodeIndex,
     ) -> Option<T>
     where
         T: for<'a> Decodable<CacheDecoder<'a, 'tcx>>,
     {
-        let opt_value = self.load_indexed(tcx, dep_node_index, &self.query_result_index);
-        debug_assert_eq!(opt_value.is_some(), self.loadable_from_disk(dep_node_index));
-        opt_value
+        self.load_indexed(tcx, dep_node_index, &self.query_values_index)
     }
 
     fn load_indexed<'tcx, T>(
@@ -815,6 +788,9 @@ pub struct CacheEncoder<'a, 'tcx> {
     hygiene_context: &'a HygieneEncodeContext,
     // Used for both `Symbol`s and `ByteSymbol`s.
     symbol_index_table: FxHashMap<u32, usize>,
+
+    query_values_index: Vec<(SerializedDepNodeIndex, AbsoluteBytePos)>,
+    side_effects_index: Vec<(SerializedDepNodeIndex, AbsoluteBytePos)>,
 }
 
 impl<'a, 'tcx> fmt::Debug for CacheEncoder<'a, 'tcx> {
@@ -835,7 +811,7 @@ impl<'a, 'tcx> CacheEncoder<'a, 'tcx> {
     /// encode the specified tag, then the given value, then the number of
     /// bytes taken up by tag and value. On decoding, we can then verify that
     /// we get the expected tag and read the expected number of bytes.
-    pub fn encode_tagged<T: Encodable<Self>, V: Encodable<Self>>(&mut self, tag: T, value: &V) {
+    fn encode_tagged<T: Encodable<Self>, V: Encodable<Self>>(&mut self, tag: T, value: &V) {
         let start_pos = self.position();
 
         tag.encode(self);
@@ -845,6 +821,20 @@ impl<'a, 'tcx> CacheEncoder<'a, 'tcx> {
         ((end_pos - start_pos) as u64).encode(self);
     }
 
+    pub fn encode_query_value<V: Encodable<Self>>(&mut self, index: DepNodeIndex, value: &V) {
+        let index = SerializedDepNodeIndex::from_curr_for_serialization(index);
+
+        self.query_values_index.push((index, AbsoluteBytePos::new(self.position())));
+        self.encode_tagged(index, value);
+    }
+
+    fn encode_side_effect(&mut self, index: DepNodeIndex, side_effect: &QuerySideEffect) {
+        let index = SerializedDepNodeIndex::from_curr_for_serialization(index);
+
+        self.side_effects_index.push((index, AbsoluteBytePos::new(self.position())));
+        self.encode_tagged(index, side_effect);
+    }
+
     // copy&paste impl from rustc_metadata
     fn encode_symbol_or_byte_symbol(
         &mut self,

compiler/rustc_middle/src/query/plumbing.rs

@@ -2,13 +2,14 @@ use std::fmt;
 use std::ops::Deref;
 
 use rustc_data_structures::fingerprint::Fingerprint;
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::hash_table::HashTable;
 use rustc_data_structures::sharded::Sharded;
-use rustc_data_structures::sync::{AtomicU64, WorkerLocal};
+use rustc_data_structures::sync::{AtomicU64, Lock, WorkerLocal};
 use rustc_errors::Diag;
 use rustc_span::Span;
 
-use crate::dep_graph::{DepKind, DepNodeIndex, SerializedDepNodeIndex};
+use crate::dep_graph::{DepKind, DepNodeIndex, QuerySideEffect, SerializedDepNodeIndex};
 use crate::ich::StableHashingContext;
 use crate::queries::{ExternProviders, Providers, QueryArenas, QueryVTables, TaggedQueryKey};
 use crate::query::on_disk_cache::OnDiskCache;
@@ -151,6 +152,13 @@ pub struct QuerySystem<'tcx> {
     pub arenas: WorkerLocal<QueryArenas<'tcx>>,
     pub query_vtables: QueryVTables<'tcx>,
 
+    /// Side-effect associated with each [`DepKind::SideEffect`] node in the
+    /// current incremental-compilation session. Side effects will be written
+    /// to disk, and loaded by [`OnDiskCache`] in the next session.
+    ///
+    /// Always empty if incremental compilation is off.
+    pub side_effects: Lock<FxIndexMap<DepNodeIndex, QuerySideEffect>>,
+
     /// This provides access to the incremental compilation on-disk cache for query results.
     /// Do not access this directly. It is only meant to be used by
     /// `DepGraph::try_mark_green()` and the query infrastructure.