From f8b019fc0a38d0b110458f3ae4807ec51a05241e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 29 Aug 2022 20:52:12 -0500 Subject: [PATCH 1/2] Move most of `TyCtxtAt::$name` into a generic function --- compiler/rustc_middle/src/ty/query.rs | 48 ++++++++++++++++----------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index 5665bb866d46c..c0142e6f882da 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -102,16 +102,38 @@ impl<'tcx> TyCtxt<'tcx> { } } -/// Helper for `TyCtxtEnsure` to avoid a closure. -#[inline(always)] -fn noop(_: &T) {} - /// Helper to ensure that queries only return `Copy` types. #[inline(always)] fn copy(x: &T) -> T { *x } +fn evaluate_query<'tcx, Cache>( + tcx: TyCtxt<'tcx>, + execute_query: fn( + &'tcx dyn QueryEngine<'tcx>, + TyCtxt<'tcx>, + Span, + Cache::Key, + QueryMode, + ) -> Option, + query_cache: &Cache, + span: Span, + key: Cache::Key, + mode: QueryMode, +) -> Option +where + Cache::Stored: Copy, + Cache: QueryCache, +{ + let cached = try_get_cached(tcx, query_cache, &key, copy); + + match cached { + Ok(value) => return value, + Err(()) => (), + } +} + macro_rules! query_helper_param_ty { (DefId) => { impl IntoQueryParam }; ($K:ty) => { $K }; @@ -220,14 +242,7 @@ macro_rules! define_callbacks { let key = key.into_query_param(); opt_remap_env_constness!([$($modifiers)*][key]); - let cached = try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key, noop); - - match cached { - Ok(()) => return, - Err(()) => (), - } - - self.tcx.queries.$name(self.tcx, DUMMY_SP, key, QueryMode::Ensure); + let _ = evaluate_query(self.tcx, QueryEngine::$name, &self.tcx.query_caches.$name, DUMMY_SP, key, QueryMode::Ensure); })* } @@ -249,14 +264,7 @@ macro_rules! define_callbacks { let key = key.into_query_param(); opt_remap_env_constness!([$($modifiers)*][key]); - let cached = try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key, copy); - - match cached { - Ok(value) => return value, - Err(()) => (), - } - - self.tcx.queries.$name(self.tcx, self.span, key, QueryMode::Get).unwrap() + evaluate_query(self.tcx, QueryEngine::$name, &self.tcx.query_caches.$name, self.span, key, QueryMode::Get).unwrap() })* } From 3675fdd28ea8676fbf579a30e4cff78e4e9ae160 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 9 Sep 2022 20:46:36 -0500 Subject: [PATCH 2/2] Get rid of OnHit by switching on `QueryMode` instead of unconditionally returning Option I think `copy` may have added some overhead? unclear what's causing the perf impact. --- compiler/rustc_middle/src/ty/query.rs | 14 +++----------- compiler/rustc_query_system/src/query/plumbing.rs | 13 ++++++------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index c0142e6f882da..eb8f6a1175a90 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -102,12 +102,6 @@ impl<'tcx> TyCtxt<'tcx> { } } -/// Helper to ensure that queries only return `Copy` types. -#[inline(always)] -fn copy(x: &T) -> T { - *x -} - fn evaluate_query<'tcx, Cache>( tcx: TyCtxt<'tcx>, execute_query: fn( @@ -126,11 +120,9 @@ where Cache::Stored: Copy, Cache: QueryCache, { - let cached = try_get_cached(tcx, query_cache, &key, copy); - - match cached { - Ok(value) => return value, - Err(()) => (), + match try_get_cached(tcx, query_cache, &key, mode) { + Ok(value) => value, + Err(()) => execute_query(tcx.queries, tcx, span, key, mode), } } diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 7bbc22e8293a5..891558e314b87 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -303,24 +303,23 @@ where /// which will be used if the query is not in the cache and we need /// to compute it. #[inline] -pub fn try_get_cached<'a, CTX, C, R, OnHit>( +pub fn try_get_cached<'a, CTX, C>( tcx: CTX, cache: &'a C, key: &C::Key, - // `on_hit` can be called while holding a lock to the query cache - on_hit: OnHit, -) -> Result + mode: QueryMode, +) -> Result, ()> where C: QueryCache, + C::Stored: Copy, CTX: DepContext, - OnHit: FnOnce(&C::Stored) -> R, { cache.lookup(&key, |value, index| { if std::intrinsics::unlikely(tcx.profiler().enabled()) { tcx.profiler().query_cache_hit(index.into()); } tcx.dep_graph().read_index(index); - on_hit(value) + if matches!(mode, QueryMode::Ensure) { None } else { Some(*value) } }) } @@ -676,7 +675,7 @@ where } } -#[derive(Debug)] +#[derive(Copy, Clone, Debug)] pub enum QueryMode { Get, Ensure,