Rollup merge of #153559 - Zalathar:persist, r=jackh726

Inline and simplify some code for saving incremental data to disk

Main changes:
- Inline `encode_query_cache` and `TyCtxt::serialize_query_result_cache`
- Pull value promotion out of `OnDiskCache::drop_serialized_data`
- Panic if `on_disk_cache` is None in an incremental-only path

compiler/rustc_incremental/src/persist/save.rs

@@ -8,7 +8,7 @@
 };
 use rustc_middle::ty::TyCtxt;
 use rustc_serialize::Encodable as RustcEncodable;
-use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
+use rustc_serialize::opaque::FileEncoder;
 use rustc_session::Session;
 use tracing::debug;
 
@@ -60,13 +60,30 @@ pub(crate) fn save_dep_graph(tcx: TyCtxt<'_>) {
                 // We execute this after `incr_comp_persist_dep_graph` for the serial compiler
                 // to catch any potential query execution writing to the dep graph.
                 sess.time("incr_comp_persist_result_cache", || {
-                    // Drop the memory map so that we can remove the file and write to it.
-                    if let Some(odc) = &tcx.query_system.on_disk_cache {
-                        odc.drop_serialized_data(tcx);
-                    }
+                    // The on-disk cache struct is always present in incremental mode,
+                    // even if there was no previous session.
+                    let on_disk_cache = tcx.query_system.on_disk_cache.as_ref().unwrap();
 
-                    file_format::save_in(sess, query_cache_path, "query cache", |e| {
-                        encode_query_cache(tcx, e)
+                    // For every green dep node that has a disk-cached value from the
+                    // previous session, make sure the value is loaded into the memory
+                    // cache, so that it will be serialized as part of this session.
+                    //
+                    // This reads data from the previous session, so it needs to happen
+                    // before dropping the mmap.
+                    //
+                    // FIXME(Zalathar): This step is intended to be cheap, but still does
+                    // quite a lot of work, especially in builds with few or no changes.
+                    // Can we be smarter about how we identify values that need promotion?
+                    // Can we promote values without decoding them into the memory cache?
+                    tcx.dep_graph.exec_cache_promotions(tcx);
+
+                    // Drop the memory map so that we can remove the file and write to it.
+                    on_disk_cache.close_serialized_data_mmap();
+
+                    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)
+                        })
                     });
                 });
             },
@@ -132,10 +149,6 @@ fn encode_work_product_index(
     serialized_products.encode(encoder)
 }
 
-fn encode_query_cache(tcx: TyCtxt<'_>, encoder: FileEncoder) -> FileEncodeResult {
-    tcx.sess.time("incr_comp_serialize_result_cache", || tcx.serialize_query_result_cache(encoder))
-}
-
 /// Builds the dependency graph.
 ///
 /// This function creates the *staging dep-graph*. When the dep-graph is modified by a query

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -202,20 +202,9 @@ pub fn new_empty() -> Self {
         }
     }
 
-    /// Execute all cache promotions and release the serialized backing Mmap.
-    ///
-    /// Cache promotions require invoking queries, which needs to read the serialized data.
-    /// In order to serialize the new on-disk cache, the former on-disk cache file needs to be
-    /// deleted, hence we won't be able to refer to its memmapped data.
-    pub fn drop_serialized_data(&self, tcx: TyCtxt<'_>) {
-        // Load everything into memory so we can write it out to the on-disk
-        // cache. The vast majority of cacheable query results should already
-        // be in memory, so this should be a cheap operation.
-        // Do this *before* we clone 'latest_foreign_def_path_hashes', since
-        // loading existing queries may cause us to create new DepNodes, which
-        // may in turn end up invoking `store_foreign_def_id_hash`
-        tcx.dep_graph.exec_cache_promotions(tcx);
-
+    /// Release the serialized backing `Mmap`.
+    pub fn close_serialized_data_mmap(&self) {
+        // Obtain a write lock, and replace the mmap with None to drop it.
         *self.serialized_data.write() = None;
     }
 

compiler/rustc_middle/src/ty/context.rs

@@ -38,7 +38,6 @@
 use rustc_hir::limit::Limit;
 use rustc_hir::{self as hir, CRATE_HIR_ID, HirId, Node, TraitCandidate, find_attr};
 use rustc_index::IndexVec;
-use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
 use rustc_session::Session;
 use rustc_session::config::CrateType;
 use rustc_session::cstore::{CrateStoreDyn, Untracked};
@@ -1495,10 +1494,6 @@ pub fn with_stable_hashing_context<R>(
         f(StableHashingContext::new(self.sess, &self.untracked))
     }
 
-    pub fn serialize_query_result_cache(self, encoder: FileEncoder) -> FileEncodeResult {
-        self.query_system.on_disk_cache.as_ref().map_or(Ok(0), |c| c.serialize(self, encoder))
-    }
-
     #[inline]
     pub fn local_crate_exports_generics(self) -> bool {
         // compiler-builtins has some special treatment in codegen, which can result in confusing