self-profile: Switch to new approach for event_id generation that enables query-invocation-specific event_ids.

michaelwoerister · michaelwoerister · commit 603892d2a5d5 · 2019-12-18T15:51:31.000+01:00
diff --git a/Cargo.lock b/Cargo.lock
@@ -2021,9 +2021,9 @@ dependencies = [
 
 [[package]]
 name = "measureme"
-version = "0.5.0"
+version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c420bbc064623934620b5ab2dc0cf96451b34163329e82f95e7fa1b7b99a6ac8"
+checksum = "36dcc09c1a633097649f7d48bde3d8a61d2a43c01ce75525e31fbbc82c0fccf4"
 dependencies = [
  "byteorder",
  "memmap",
diff --git a/src/librustc/Cargo.toml b/src/librustc/Cargo.toml
@@ -37,6 +37,6 @@ byteorder = { version = "1.3" }
 chalk-engine = { version = "0.9.0", default-features=false }
 rustc_fs_util = { path = "../librustc_fs_util" }
 smallvec = { version = "1.0", features = ["union", "may_dangle"] }
-measureme = "0.5"
+measureme = "0.6.0"
 rustc_error_codes = { path = "../librustc_error_codes" }
 rustc_session = { path = "../librustc_session" }
diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs
@@ -3,9 +3,10 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_index::vec::{Idx, IndexVec};
 use smallvec::SmallVec;
+use rustc_data_structures::profiling::QueryInvocationId;
 use rustc_data_structures::sync::{Lrc, Lock, AtomicU32, AtomicU64, Ordering};
 use rustc_data_structures::sharded::{self, Sharded};
-use std::sync::atomic::Ordering::SeqCst;
+use std::sync::atomic::Ordering::Relaxed;
 use std::env;
 use std::hash::Hash;
 use std::collections::hash_map::Entry;
@@ -25,6 +26,12 @@ use super::prev::PreviousDepGraph;
 #[derive(Clone)]
 pub struct DepGraph {
     data: Option<Lrc<DepGraphData>>,
+
+    /// This field is used for assigning DepNodeIndices when running in
+    /// non-incremental mode. Even in non-incremental mode we make sure that
+    /// each task as a `DepNodeIndex` that uniquely identifies it. This unique
+    /// ID is used for self-profiling.
+    virtual_dep_node_index: Lrc<AtomicU32>,
 }
 
 rustc_index::newtype_index! {
@@ -35,6 +42,13 @@ impl DepNodeIndex {
     pub const INVALID: DepNodeIndex = DepNodeIndex::MAX;
 }
 
+impl std::convert::From<DepNodeIndex> for QueryInvocationId {
+    #[inline]
+    fn from(dep_node_index: DepNodeIndex) -> Self {
+         QueryInvocationId(dep_node_index.as_u32())
+    }
+}
+
 #[derive(PartialEq)]
 pub enum DepNodeColor {
     Red,
@@ -103,12 +117,14 @@ impl DepGraph {
                 previous: prev_graph,
                 colors: DepNodeColorMap::new(prev_graph_node_count),
             })),
+            virtual_dep_node_index: Lrc::new(AtomicU32::new(0)),
         }
     }
 
     pub fn new_disabled() -> DepGraph {
         DepGraph {
             data: None,
+            virtual_dep_node_index: Lrc::new(AtomicU32::new(0)),
         }
     }
 
@@ -319,7 +335,7 @@ impl DepGraph {
 
             (result, dep_node_index)
         } else {
-            (task(cx, arg), DepNodeIndex::INVALID)
+            (task(cx, arg), self.next_virtual_depnode_index())
         }
     }
 
@@ -354,7 +370,7 @@ impl DepGraph {
                                      .complete_anon_task(dep_kind, task_deps);
             (result, dep_node_index)
         } else {
-            (op(), DepNodeIndex::INVALID)
+            (op(), self.next_virtual_depnode_index())
         }
     }
 
@@ -877,6 +893,11 @@ impl DepGraph {
             }
         }
     }
+
+    fn next_virtual_depnode_index(&self) -> DepNodeIndex {
+        let index = self.virtual_dep_node_index.fetch_add(1, Relaxed);
+        DepNodeIndex::from_u32(index)
+    }
 }
 
 /// A "work product" is an intermediate result that we save into the
diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs
@@ -3,7 +3,7 @@ use crate::dep_graph::{DepKind, DepNode};
 use crate::hir::def_id::{CrateNum, DefId};
 use crate::ty::TyCtxt;
 use crate::ty::query::queries;
-use crate::ty::query::{Query, QueryName};
+use crate::ty::query::{Query};
 use crate::ty::query::QueryCache;
 use crate::ty::query::plumbing::CycleError;
 use rustc_data_structures::profiling::ProfileCategory;
@@ -20,7 +20,7 @@ use crate::ich::StableHashingContext;
 // FIXME(eddyb) false positive, the lifetime parameter is used for `Key`/`Value`.
 #[allow(unused_lifetimes)]
 pub trait QueryConfig<'tcx> {
-    const NAME: QueryName;
+    const NAME: &'static str;
     const CATEGORY: ProfileCategory;
 
     type Key: Eq + Hash + Clone + Debug;
diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs
@@ -104,7 +104,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {
             if let Some((_, value)) =
                 lock.results.raw_entry().from_key_hashed_nocheck(key_hash, key)
             {
-                tcx.prof.query_cache_hit(Q::NAME);
+                tcx.prof.query_cache_hit(value.index.into());
                 let result = (value.value.clone(), value.index);
                 #[cfg(debug_assertions)]
                 {
@@ -356,7 +356,7 @@ impl<'tcx> TyCtxt<'tcx> {
     #[inline(never)]
     pub(super) fn get_query<Q: QueryDescription<'tcx>>(self, span: Span, key: Q::Key) -> Q::Value {
         debug!("ty::query::get_query<{}>(key={:?}, span={:?})",
-               Q::NAME.as_str(),
+               Q::NAME,
                key,
                span);
 
@@ -378,7 +378,7 @@ impl<'tcx> TyCtxt<'tcx> {
 
         if Q::ANON {
 
-            let prof_timer = self.prof.query_provider(Q::NAME);
+            let prof_timer = self.prof.query_provider();
 
             let ((result, dep_node_index), diagnostics) = with_diagnostics(|diagnostics| {
                 self.start_query(job.job.clone(), diagnostics, |tcx| {
@@ -388,7 +388,7 @@ impl<'tcx> TyCtxt<'tcx> {
                 })
             });
 
-            drop(prof_timer);
+            prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
             self.dep_graph.read_index(dep_node_index);
 
@@ -445,8 +445,9 @@ impl<'tcx> TyCtxt<'tcx> {
         // First we try to load the result from the on-disk cache.
         let result = if Q::cache_on_disk(self, key.clone(), None) &&
                         self.sess.opts.debugging_opts.incremental_queries {
-            let _prof_timer = self.prof.incr_cache_loading(Q::NAME);
+            let prof_timer = self.prof.incr_cache_loading();
             let result = Q::try_load_from_disk(self, prev_dep_node_index);
+            prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
             // We always expect to find a cached result for things that
             // can be forced from `DepNode`.
@@ -465,13 +466,15 @@ impl<'tcx> TyCtxt<'tcx> {
         } else {
             // We could not load a result from the on-disk cache, so
             // recompute.
-            let _prof_timer = self.prof.query_provider(Q::NAME);
+            let prof_timer = self.prof.query_provider();
 
             // The dep-graph for this computation is already in-place.
             let result = self.dep_graph.with_ignore(|| {
                 Q::compute(self, key)
             });
 
+            prof_timer.finish_with_query_invocation_id(dep_node_index.into());
+
             result
         };
 
@@ -534,7 +537,7 @@ impl<'tcx> TyCtxt<'tcx> {
                  - dep-node: {:?}",
                 key, dep_node);
 
-        let prof_timer = self.prof.query_provider(Q::NAME);
+        let prof_timer = self.prof.query_provider();
 
         let ((result, dep_node_index), diagnostics) = with_diagnostics(|diagnostics| {
             self.start_query(job.job.clone(), diagnostics, |tcx| {
@@ -554,7 +557,7 @@ impl<'tcx> TyCtxt<'tcx> {
             })
         });
 
-        drop(prof_timer);
+        prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
         if unlikely!(!diagnostics.is_empty()) {
             if dep_node.kind != crate::dep_graph::DepKind::Null {
@@ -586,17 +589,19 @@ impl<'tcx> TyCtxt<'tcx> {
 
         let dep_node = Q::to_dep_node(self, &key);
 
-        if self.dep_graph.try_mark_green_and_read(self, &dep_node).is_none() {
-            // A None return from `try_mark_green_and_read` means that this is either
-            // a new dep node or that the dep node has already been marked red.
-            // Either way, we can't call `dep_graph.read()` as we don't have the
-            // DepNodeIndex. We must invoke the query itself. The performance cost
-            // this introduces should be negligible as we'll immediately hit the
-            // in-memory cache, or another query down the line will.
-
-            let _ = self.get_query::<Q>(DUMMY_SP, key);
-        } else {
-            self.prof.query_cache_hit(Q::NAME);
+        match self.dep_graph.try_mark_green_and_read(self, &dep_node) {
+            None => {
+                // A None return from `try_mark_green_and_read` means that this is either
+                // a new dep node or that the dep node has already been marked red.
+                // Either way, we can't call `dep_graph.read()` as we don't have the
+                // DepNodeIndex. We must invoke the query itself. The performance cost
+                // this introduces should be negligible as we'll immediately hit the
+                // in-memory cache, or another query down the line will.
+                let _ = self.get_query::<Q>(DUMMY_SP, key);
+            }
+            Some((_, dep_node_index)) => {
+                self.prof.query_cache_hit(dep_node_index.into());
+            }
         }
     }
 
@@ -713,6 +718,42 @@ macro_rules! define_queries_inner {
                 }
             }
 
+            /// All self-profiling events generated by the query engine use a
+            /// virtual `StringId`s for their `event_id`. This method makes all
+            /// those virtual `StringId`s point to actual strings.
+            ///
+            /// If we are recording only summary data, the ids will point to
+            /// just the query names. If we are recording query keys too, we
+            /// allocate the corresponding strings here. (The latter is not yet
+            /// implemented.)
+            pub fn allocate_self_profile_query_strings(
+                &self,
+                profiler: &rustc_data_structures::profiling::SelfProfiler
+            ) {
+                // Walk the entire query cache and allocate the appropriate
+                // string representation. Each cache entry is uniquely
+                // identified by its dep_node_index.
+                $({
+                    let query_name_string_id =
+                        profiler.get_or_alloc_cached_string(stringify!($name));
+
+                    let result_cache = self.$name.lock_shards();
+
+                    for shard in result_cache.iter() {
+                        let query_invocation_ids = shard
+                            .results
+                            .values()
+                            .map(|v| v.index)
+                            .map(|dep_node_index| dep_node_index.into());
+
+                        profiler.bulk_map_query_invocation_id_to_single_string(
+                            query_invocation_ids,
+                            query_name_string_id
+                        );
+                    }
+                })*
+            }
+
             #[cfg(parallel_compiler)]
             pub fn collect_active_jobs(&self) -> Vec<Lrc<QueryJob<$tcx>>> {
                 let mut jobs = Vec::new();
@@ -830,36 +871,6 @@ macro_rules! define_queries_inner {
             }
         }
 
-        #[allow(nonstandard_style)]
-        #[derive(Clone, Copy)]
-        pub enum QueryName {
-            $($name),*
-        }
-
-        impl rustc_data_structures::profiling::QueryName for QueryName {
-            fn discriminant(self) -> std::mem::Discriminant<QueryName> {
-                std::mem::discriminant(&self)
-            }
-
-            fn as_str(self) -> &'static str {
-                QueryName::as_str(&self)
-            }
-        }
-
-        impl QueryName {
-            pub fn register_with_profiler(
-                profiler: &rustc_data_structures::profiling::SelfProfiler,
-            ) {
-                $(profiler.register_query_name(QueryName::$name);)*
-            }
-
-            pub fn as_str(&self) -> &'static str {
-                match self {
-                    $(QueryName::$name => stringify!($name),)*
-                }
-            }
-        }
-
         #[allow(nonstandard_style)]
         #[derive(Clone, Debug)]
         pub enum Query<$tcx> {
@@ -900,12 +911,6 @@ macro_rules! define_queries_inner {
                     $(Query::$name(key) => key.default_span(tcx),)*
                 }
             }
-
-            pub fn query_name(&self) -> QueryName {
-                match self {
-                    $(Query::$name(_) => QueryName::$name,)*
-                }
-            }
         }
 
         impl<'a, $tcx> HashStable<StableHashingContext<'a>> for Query<$tcx> {
@@ -940,7 +945,7 @@ macro_rules! define_queries_inner {
             type Key = $K;
             type Value = $V;
 
-            const NAME: QueryName = QueryName::$name;
+            const NAME: &'static str = stringify!($name);
             const CATEGORY: ProfileCategory = $category;
         }
 
diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs
@@ -502,7 +502,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
 
         ongoing_codegen.codegen_finished(tcx);
 
-        assert_and_save_dep_graph(tcx);
+        finalize_tcx(tcx);
 
         ongoing_codegen.check_for_errors(tcx.sess);
 
@@ -647,7 +647,8 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
 
     ongoing_codegen.check_for_errors(tcx.sess);
 
-    assert_and_save_dep_graph(tcx);
+    finalize_tcx(tcx);
+
     ongoing_codegen.into_inner()
 }
 
@@ -698,14 +699,22 @@ impl<B: ExtraBackendMethods> Drop for AbortCodegenOnDrop<B> {
     }
 }
 
-fn assert_and_save_dep_graph(tcx: TyCtxt<'_>) {
+fn finalize_tcx(tcx: TyCtxt<'_>) {
     time(tcx.sess,
          "assert dep graph",
          || ::rustc_incremental::assert_dep_graph(tcx));
 
     time(tcx.sess,
          "serialize dep graph",
          || ::rustc_incremental::save_dep_graph(tcx));
+
+    // We assume that no queries are run past here. If there are new queries
+    // after this point, they'll show up as "<unknown>" in self-profiling data.
+    tcx.prof.with_profiler(|profiler| {
+        let _prof_timer =
+            tcx.prof.generic_activity("self_profile_alloc_query_strings");
+        tcx.queries.allocate_self_profile_query_strings(profiler);
+    });
 }
 
 impl CrateInfo {
diff --git a/src/librustc_data_structures/Cargo.toml b/src/librustc_data_structures/Cargo.toml
@@ -26,7 +26,7 @@ rustc-hash = "1.0.1"
 smallvec = { version = "1.0", features = ["union", "may_dangle"] }
 rustc_index = { path = "../librustc_index", package = "rustc_index" }
 bitflags = "1.2.1"
-measureme = "0.5"
+measureme = "0.6.0"
 
 [dependencies.parking_lot]
 version = "0.9"
diff --git a/src/librustc_data_structures/profiling.rs b/src/librustc_data_structures/profiling.rs
diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs

Original file line number	Diff line number	Diff line change
`@@ -3,9 +3,10 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};`
`3`	`3`	`use rustc_data_structures::fx::{FxHashMap, FxHashSet};`
`4`	`4`	`use rustc_index::vec::{Idx, IndexVec};`
`5`	`5`	`use smallvec::SmallVec;`
	`6`	`+use rustc_data_structures::profiling::QueryInvocationId;`
`6`	`7`	`use rustc_data_structures::sync::{Lrc, Lock, AtomicU32, AtomicU64, Ordering};`
`7`	`8`	`use rustc_data_structures::sharded::{self, Sharded};`
`8`		`-use std::sync::atomic::Ordering::SeqCst;`
	`9`	`+use std::sync::atomic::Ordering::Relaxed;`
`9`	`10`	`use std::env;`
`10`	`11`	`use std::hash::Hash;`
`11`	`12`	`use std::collections::hash_map::Entry;`
`@@ -25,6 +26,12 @@ use super::prev::PreviousDepGraph;`
`25`	`26`	`#[derive(Clone)]`
`26`	`27`	`pub struct DepGraph {`
`27`	`28`	`data: Option<Lrc<DepGraphData>>,`
	`29`	`+`
	`30`	`+ /// This field is used for assigning DepNodeIndices when running in`
	`31`	`+ /// non-incremental mode. Even in non-incremental mode we make sure that`
	`32`	+ /// each task as a `DepNodeIndex` that uniquely identifies it. This unique
	`33`	`+ /// ID is used for self-profiling.`
	`34`	`+ virtual_dep_node_index: Lrc<AtomicU32>,`
`28`	`35`	`}`
`29`	`36`
`30`	`37`	`rustc_index::newtype_index! {`
`@@ -35,6 +42,13 @@ impl DepNodeIndex {`
`35`	`42`	`pub const INVALID: DepNodeIndex = DepNodeIndex::MAX;`
`36`	`43`	`}`
`37`	`44`
	`45`	`+impl std::convert::From<DepNodeIndex> for QueryInvocationId {`
	`46`	`+ #[inline]`
	`47`	`+ fn from(dep_node_index: DepNodeIndex) -> Self {`
	`48`	`+ QueryInvocationId(dep_node_index.as_u32())`
	`49`	`+ }`
	`50`	`+}`
	`51`	`+`
`38`	`52`	`#[derive(PartialEq)]`
`39`	`53`	`pub enum DepNodeColor {`
`40`	`54`	`Red,`
`@@ -103,12 +117,14 @@ impl DepGraph {`
`103`	`117`	`previous: prev_graph,`
`104`	`118`	`colors: DepNodeColorMap::new(prev_graph_node_count),`
`105`	`119`	`})),`
	`120`	`+ virtual_dep_node_index: Lrc::new(AtomicU32::new(0)),`
`106`	`121`	`}`
`107`	`122`	`}`
`108`	`123`
`109`	`124`	`pub fn new_disabled() -> DepGraph {`
`110`	`125`	`DepGraph {`
`111`	`126`	`data: None,`
	`127`	`+ virtual_dep_node_index: Lrc::new(AtomicU32::new(0)),`
`112`	`128`	`}`
`113`	`129`	`}`
`114`	`130`
`@@ -319,7 +335,7 @@ impl DepGraph {`
`319`	`335`
`320`	`336`	`(result, dep_node_index)`
`321`	`337`	`} else {`
`322`		`- (task(cx, arg), DepNodeIndex::INVALID)`
	`338`	`+ (task(cx, arg), self.next_virtual_depnode_index())`
`323`	`339`	`}`
`324`	`340`	`}`
`325`	`341`
`@@ -354,7 +370,7 @@ impl DepGraph {`
`354`	`370`	`.complete_anon_task(dep_kind, task_deps);`
`355`	`371`	`(result, dep_node_index)`
`356`	`372`	`} else {`
`357`		`- (op(), DepNodeIndex::INVALID)`
	`373`	`+ (op(), self.next_virtual_depnode_index())`
`358`	`374`	`}`
`359`	`375`	`}`
`360`	`376`
`@@ -877,6 +893,11 @@ impl DepGraph {`
`877`	`893`	`}`
`878`	`894`	`}`
`879`	`895`	`}`
	`896`	`+`
	`897`	`+ fn next_virtual_depnode_index(&self) -> DepNodeIndex {`
	`898`	`+ let index = self.virtual_dep_node_index.fetch_add(1, Relaxed);`
	`899`	`+ DepNodeIndex::from_u32(index)`
	`900`	`+ }`
`880`	`901`	`}`
`881`	`902`
`882`	`903`	`/// A "work product" is an intermediate result that we save into the`