From b164dbc2715aa15be5c9f363a00d71e0847b2e77 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 22:20:17 -0500 Subject: Don't create a new `try_load_from_disk` closure for each query Instead, define a single function, parameterized only by the return type. --- compiler/rustc_query_impl/src/lib.rs | 3 ++- compiler/rustc_query_impl/src/plumbing.rs | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) (limited to 'compiler/rustc_query_impl/src') diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index 26d397f70e0..8148f8e017c 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -17,7 +17,7 @@ extern crate rustc_middle; use rustc_data_structures::sync::AtomicU64; use rustc_middle::arena::Arena; -use rustc_middle::dep_graph::{self, DepKindStruct, SerializedDepNodeIndex}; +use rustc_middle::dep_graph::{self, DepKindStruct}; use rustc_middle::ty::query::{query_keys, query_storage, query_stored, query_values}; use rustc_middle::ty::query::{ExternProviders, Providers, QueryEngine}; use rustc_middle::ty::{self, TyCtxt}; @@ -34,6 +34,7 @@ pub use rustc_query_system::query::{deadlock, QueryContext}; mod keys; use keys::Key; +use rustc_query_system::dep_graph::SerializedDepNodeIndex; pub use rustc_query_system::query::QueryConfig; pub(crate) use rustc_query_system::query::{QueryDescription, QueryVTable}; diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 1e375deb20d..48539d580c7 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -3,6 +3,7 @@ //! manage the caches, and so forth. use crate::keys::Key; +use crate::on_disk_cache::CacheDecoder; use crate::{on_disk_cache, Queries}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync::Lock; @@ -19,6 +20,7 @@ use rustc_query_system::query::{ QuerySideEffects, QueryStackFrame, }; use rustc_query_system::Value; +use rustc_serialize::Decodable; use std::any::Any; use std::num::NonZeroU64; use thin_vec::ThinVec; @@ -253,6 +255,18 @@ macro_rules! get_provider { }; } +macro_rules! should_ever_cache_on_disk { + ([]) => {{ + None + }}; + ([(cache) $($rest:tt)*]) => {{ + Some($crate::plumbing::try_load_from_disk::) + }}; + ([$other:tt $($modifiers:tt)*]) => { + should_ever_cache_on_disk!([$($modifiers)*]) + }; +} + pub(crate) fn create_query_frame< 'tcx, K: Copy + Key + for<'a> HashStable>, @@ -313,6 +327,16 @@ where } } +pub(crate) fn try_load_from_disk<'tcx, V>( + tcx: QueryCtxt<'tcx>, + id: SerializedDepNodeIndex, +) -> Option +where + V: for<'a> Decodable>, +{ + tcx.on_disk_cache().as_ref()?.try_load_query_result(*tcx, id) +} + fn force_from_dep_node<'tcx, Q>(tcx: TyCtxt<'tcx>, dep_node: DepNode) -> bool where Q: QueryDescription>, -- cgit 1.4.1-3-g733a5 From 7208bdee33460b9915e6b389b236d231d2ca3ffc Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 22:26:03 -0500 Subject: Remove `cache_on_disk` from `QueryVTable` This is not only simpler, but removes a generic function and unwrap. I have hope it will see compile time and bootstrap time improvements. --- compiler/rustc_query_impl/src/plumbing.rs | 3 +-- compiler/rustc_query_system/src/query/config.rs | 10 ++-------- compiler/rustc_query_system/src/query/plumbing.rs | 6 +++--- 3 files changed, 6 insertions(+), 13 deletions(-) (limited to 'compiler/rustc_query_impl/src') diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 48539d580c7..4ff3917e113 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -442,8 +442,7 @@ macro_rules! define_queries { hash_result: hash_result!([$($modifiers)*]), handle_cycle_error: handle_cycle_error!([$($modifiers)*]), compute, - cache_on_disk, - try_load_from_disk: Self::TRY_LOAD_FROM_DISK, + try_load_from_disk: if cache_on_disk { Self::TRY_LOAD_FROM_DISK } else { None }, } } diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index c63e110a62e..340deb88915 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -25,11 +25,12 @@ pub struct QueryVTable { pub dep_kind: CTX::DepKind, pub eval_always: bool, pub depth_limit: bool, - pub cache_on_disk: bool, pub compute: fn(CTX::DepContext, K) -> V, pub hash_result: Option, &V) -> Fingerprint>, pub handle_cycle_error: HandleCycleError, + // NOTE: this is not quite the same as `Q::TRY_LOAD_FROM_DISK`; it can also be `None` if + // `cache_on_disk` returned false for this key. pub try_load_from_disk: Option Option>, } @@ -44,13 +45,6 @@ impl QueryVTable { pub(crate) fn compute(&self, tcx: CTX::DepContext, key: K) -> V { (self.compute)(tcx, key) } - - pub(crate) fn try_load_from_disk(&self, tcx: CTX, index: SerializedDepNodeIndex) -> Option { - self.try_load_from_disk - .expect("QueryDescription::load_from_disk() called for an unsupported query.")( - tcx, index, - ) - } } pub trait QueryDescription: QueryConfig { diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index e39e39860cb..8179a674afa 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -488,14 +488,14 @@ where // First we try to load the result from the on-disk cache. // Some things are never cached on disk. - if query.cache_on_disk { + if let Some(try_load_from_disk) = query.try_load_from_disk { let prof_timer = tcx.dep_context().profiler().incr_cache_loading(); // The call to `with_query_deserialization` enforces that no new `DepNodes` // are created during deserialization. See the docs of that method for more // details. - let result = dep_graph - .with_query_deserialization(|| query.try_load_from_disk(tcx, prev_dep_node_index)); + let result = + dep_graph.with_query_deserialization(|| try_load_from_disk(tcx, prev_dep_node_index)); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); -- cgit 1.4.1-3-g733a5 From 9273782d559a342beb2443018f2f8fc873f53b79 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 5 Sep 2022 06:43:11 -0500 Subject: Move `TRY_LOAD_FROM_DISK` out of `rustc_queries` to `rustc_query_impl` We want to refer to `crate::plumbing::try_load_from_disk` in the const, but hard-coding it in rustc_queries, where we don't yet know the crate this macro will be called in, seems kind of hacky. Do it in query_impl instead. --- compiler/rustc_macros/src/query.rs | 9 ++++----- compiler/rustc_query_impl/src/plumbing.rs | 3 +++ 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'compiler/rustc_query_impl/src') diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index 55d23ba1c67..742ae964f5c 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -253,9 +253,6 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea fn cache_on_disk(#tcx: TyCtxt<'tcx>, #key: &Self::Key) -> bool { #expr } - - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = Some(crate::plumbing::try_load_from_disk::); } } else { quote! { @@ -263,8 +260,6 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea fn cache_on_disk(_: TyCtxt<'tcx>, _: &Self::Key) -> bool { false } - - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> = None; } }; @@ -333,6 +328,10 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream { remap_env_constness, ); + if modifiers.cache.is_some() { + attributes.push(quote! { (cache) }); + } + // This uses the span of the query definition for the commas, // which can be important if we later encounter any ambiguity // errors with any of the numerous macro_rules! macros that diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 4ff3917e113..e38de2a0e40 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -412,6 +412,9 @@ macro_rules! define_queries { impl<'tcx> QueryDescription> for queries::$name<'tcx> { rustc_query_description! { $name } + const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> + = should_ever_cache_on_disk!([$($modifiers)*]); + type Cache = query_storage::$name<'tcx>; #[inline(always)] -- cgit 1.4.1-3-g733a5 From 0a9d7dbca23bb05fab13c1dc75a87cdb2bf69ff5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 6 Sep 2022 19:09:32 -0500 Subject: Remove unnecessary `TRY_LOAD_FROM_DISK` constant --- compiler/rustc_query_impl/src/lib.rs | 1 - compiler/rustc_query_impl/src/plumbing.rs | 5 +---- compiler/rustc_query_system/src/query/config.rs | 5 +---- 3 files changed, 2 insertions(+), 9 deletions(-) (limited to 'compiler/rustc_query_impl/src') diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index 8148f8e017c..c87d26b3950 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -34,7 +34,6 @@ pub use rustc_query_system::query::{deadlock, QueryContext}; mod keys; use keys::Key; -use rustc_query_system::dep_graph::SerializedDepNodeIndex; pub use rustc_query_system::query::QueryConfig; pub(crate) use rustc_query_system::query::{QueryDescription, QueryVTable}; diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index e38de2a0e40..6e0649cc471 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -412,9 +412,6 @@ macro_rules! define_queries { impl<'tcx> QueryDescription> for queries::$name<'tcx> { rustc_query_description! { $name } - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = should_ever_cache_on_disk!([$($modifiers)*]); - type Cache = query_storage::$name<'tcx>; #[inline(always)] @@ -445,7 +442,7 @@ macro_rules! define_queries { hash_result: hash_result!([$($modifiers)*]), handle_cycle_error: handle_cycle_error!([$($modifiers)*]), compute, - try_load_from_disk: if cache_on_disk { Self::TRY_LOAD_FROM_DISK } else { None }, + try_load_from_disk: if cache_on_disk { should_ever_cache_on_disk!([$($modifiers)*]) } else { None }, } } diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index 340deb88915..c4549cc9eb4 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -29,8 +29,7 @@ pub struct QueryVTable { pub compute: fn(CTX::DepContext, K) -> V, pub hash_result: Option, &V) -> Fingerprint>, pub handle_cycle_error: HandleCycleError, - // NOTE: this is not quite the same as `Q::TRY_LOAD_FROM_DISK`; it can also be `None` if - // `cache_on_disk` returned false for this key. + // NOTE: this is also `None` if `cache_on_disk()` returns false, not just if it's unsupported by the query pub try_load_from_disk: Option Option>, } @@ -48,8 +47,6 @@ impl QueryVTable { } pub trait QueryDescription: QueryConfig { - const TRY_LOAD_FROM_DISK: Option Option>; - type Cache: QueryCache; fn describe(tcx: CTX, key: Self::Key) -> String; -- cgit 1.4.1-3-g733a5