From e60ccfc6a9e91a7b26f368c9fee40bd22831450f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 31 Jan 2023 18:15:42 +0100 Subject: [PATCH 1/2] Don't inline query_cache_hit to reduce code size of the query hot path. --- compiler/rustc_data_structures/src/profiling.rs | 2 +- compiler/rustc_query_system/src/query/plumbing.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index 393f1739081..20827c9fd1a 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -393,7 +393,7 @@ impl SelfProfilerRef { } /// Record a query in-memory cache hit. - #[inline(always)] + #[inline(never)] pub fn query_cache_hit(&self, query_invocation_id: QueryInvocationId) { self.instant_query_event( |profiler| profiler.query_cache_hit_event_kind, diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index ffc413d15f5..2fde3c6075b 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -722,7 +722,9 @@ where } Some((_, dep_node_index)) => { dep_graph.read_index(dep_node_index); - qcx.dep_context().profiler().query_cache_hit(dep_node_index.into()); + if std::intrinsics::unlikely(qcx.dep_context().profiler().enabled()) { + qcx.dep_context().profiler().query_cache_hit(dep_node_index.into()); + } (false, None) } } From 9539737008852fffc3067dbfaa96a0ca0603b1b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 1 Feb 2023 04:49:09 +0100 Subject: [PATCH 2/2] Make an optimal cold path for query_cache_hit --- compiler/rustc_data_structures/src/lib.rs | 1 + .../rustc_data_structures/src/profiling.rs | 41 ++++++++++--------- .../rustc_query_system/src/dep_graph/graph.rs | 2 +- .../rustc_query_system/src/query/plumbing.rs | 16 ++------ 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_data_structures/src/lib.rs b/compiler/rustc_data_structures/src/lib.rs index 954e84c303b..7fab8954cb1 100644 --- a/compiler/rustc_data_structures/src/lib.rs +++ b/compiler/rustc_data_structures/src/lib.rs @@ -11,6 +11,7 @@ #![feature(associated_type_bounds)] #![feature(auto_traits)] #![feature(cell_leak)] +#![feature(core_intrinsics)] #![feature(extend_one)] #![feature(hash_raw_entry)] #![feature(hasher_prefixfree_extras)] diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index 20827c9fd1a..3aca03f6e5c 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -88,6 +88,7 @@ use std::borrow::Borrow; use std::collections::hash_map::Entry; use std::error::Error; use std::fs; +use std::intrinsics::unlikely; use std::path::Path; use std::process; use std::sync::Arc; @@ -393,13 +394,20 @@ impl SelfProfilerRef { } /// Record a query in-memory cache hit. - #[inline(never)] + #[inline(always)] pub fn query_cache_hit(&self, query_invocation_id: QueryInvocationId) { - self.instant_query_event( - |profiler| profiler.query_cache_hit_event_kind, - query_invocation_id, - EventFilter::QUERY_CACHE_HITS, - ); + #[inline(never)] + #[cold] + fn cold_call(profiler_ref: &SelfProfilerRef, query_invocation_id: QueryInvocationId) { + profiler_ref.instant_query_event( + |profiler| profiler.query_cache_hit_event_kind, + query_invocation_id, + ); + } + + if unlikely(self.event_filter_mask.contains(EventFilter::QUERY_CACHE_HITS)) { + cold_call(self, query_invocation_id); + } } /// Start profiling a query being blocked on a concurrent execution. @@ -444,20 +452,15 @@ impl SelfProfilerRef { &self, event_kind: fn(&SelfProfiler) -> StringId, query_invocation_id: QueryInvocationId, - event_filter: EventFilter, ) { - drop(self.exec(event_filter, |profiler| { - let event_id = StringId::new_virtual(query_invocation_id.0); - let thread_id = get_thread_id(); - - profiler.profiler.record_instant_event( - event_kind(profiler), - EventId::from_virtual(event_id), - thread_id, - ); - - TimingGuard::none() - })); + let event_id = StringId::new_virtual(query_invocation_id.0); + let thread_id = get_thread_id(); + let profiler = self.profiler.as_ref().unwrap(); + profiler.profiler.record_instant_event( + event_kind(profiler), + EventId::from_virtual(event_id), + thread_id, + ); } pub fn with_profiler(&self, f: impl FnOnce(&SelfProfiler)) { diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 47b2fd8f8f4..9443ded704d 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -47,7 +47,7 @@ impl DepNodeIndex { } impl From for QueryInvocationId { - #[inline] + #[inline(always)] fn from(dep_node_index: DepNodeIndex) -> Self { QueryInvocationId(dep_node_index.as_u32()) } diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 2fde3c6075b..f59d71124ec 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -346,9 +346,7 @@ where { match cache.lookup(&key) { Some((value, index)) => { - if std::intrinsics::unlikely(tcx.profiler().enabled()) { - tcx.profiler().query_cache_hit(index.into()); - } + tcx.profiler().query_cache_hit(index.into()); tcx.dep_graph().read_index(index); Some(value) } @@ -408,9 +406,7 @@ where panic!("value must be in cache after waiting") }; - if std::intrinsics::unlikely(qcx.dep_context().profiler().enabled()) { - qcx.dep_context().profiler().query_cache_hit(index.into()); - } + qcx.dep_context().profiler().query_cache_hit(index.into()); query_blocked_prof_timer.finish_with_query_invocation_id(index.into()); (v, Some(index)) @@ -722,9 +718,7 @@ where } Some((_, dep_node_index)) => { dep_graph.read_index(dep_node_index); - if std::intrinsics::unlikely(qcx.dep_context().profiler().enabled()) { - qcx.dep_context().profiler().query_cache_hit(dep_node_index.into()); - } + qcx.dep_context().profiler().query_cache_hit(dep_node_index.into()); (false, None) } } @@ -778,9 +772,7 @@ where // Ensure that only one of them runs the query. let cache = Q::query_cache(qcx); if let Some((_, index)) = cache.lookup(&key) { - if std::intrinsics::unlikely(qcx.dep_context().profiler().enabled()) { - qcx.dep_context().profiler().query_cache_hit(index.into()); - } + qcx.dep_context().profiler().query_cache_hit(index.into()); return; }