Make the marker category part of the schema.

Author: mstange

fxprof-processed-profile/src/fast_hash_map.rs

@@ -1,5 +1,5 @@
-use rustc_hash::FxHashMap;
 use indexmap::IndexSet;
+use rustc_hash::FxHashMap;
 
 pub type FastHashMap<K, V> = FxHashMap<K, V>;
 pub type FastIndexSet<V> = IndexSet<V, rustc_hash::FxBuildHasher>;

fxprof-processed-profile/src/marker_table.rs

@@ -58,7 +58,6 @@ impl MarkerTable {
         schema: &InternalMarkerSchema,
         marker: T,
         timing: MarkerTiming,
-        category: CategoryHandle,
         thread_string_table: &mut ThreadStringTable,
         global_string_table: &mut GlobalStringTable,
     ) -> MarkerHandle {
@@ -68,7 +67,7 @@ impl MarkerTable {
             MarkerTiming::IntervalStart(s) => (Some(s), None, Phase::IntervalStart),
             MarkerTiming::IntervalEnd(e) => (None, Some(e), Phase::IntervalEnd),
         };
-        self.marker_categories.push(category);
+        self.marker_categories.push(schema.category());
         self.marker_name_string_indexes.push(name_string_index);
         self.marker_starts.push(s);
         self.marker_ends.push(e);

fxprof-processed-profile/src/markers.rs

@@ -8,7 +8,7 @@ use serde_derive::Serialize;
 
 use super::profile::StringHandle;
 use super::timestamp::Timestamp;
-use crate::{CategoryHandle, Profile};
+use crate::{Category, CategoryHandle, Profile};
 
 /// The handle for a marker. Returned from [`Profile::add_marker`].
 ///
@@ -65,9 +65,6 @@ pub trait Marker {
     /// used as `{marker.name}` in the various `label` template strings in the schema.
     fn name(&self, profile: &mut Profile) -> StringHandle;
 
-    /// The category of this marker. The marker chart groups marker rows by category.
-    fn category(&self, profile: &mut Profile) -> CategoryHandle;
-
     /// Called for any fields defined in the schema whose [`format`](RuntimeSchemaMarkerField::format) is
     /// of [kind](MarkerFieldFormat::kind) [`MarkerFieldFormatKind::String`].
     ///
@@ -98,17 +95,15 @@ pub trait Marker {
 /// The trait for markers whose schema is known at compile time. Any type which implements
 /// [`StaticSchemaMarker`] automatically implements the [`Marker`] trait via a blanket impl.
 ///
-/// Markers have a type, a name, a category, and an arbitrary number of fields.
+/// Markers have a type, a name, and an arbitrary number of fields.
 /// The fields of a marker type are defined by the marker type's schema, see [`RuntimeSchemaMarkerSchema`].
 /// The timestamps are not part of the marker; they are supplied separately to
 /// [`Profile::add_marker`] when a marker is added to the profile.
 ///
-/// In [`StaticSchemaMarker`], the schema is returned from a static `schema` method.
-///
 /// ```
 /// use fxprof_processed_profile::{
 ///     Profile, Marker, MarkerLocations, MarkerFieldFlags, MarkerFieldFormat, StaticSchemaMarkerField,
-///     StaticSchemaMarker, CategoryHandle, StringHandle,
+///     StaticSchemaMarker, Category, CategoryColor, StringHandle,
 /// };
 ///
 /// /// An example marker type with a name and some text content.
@@ -121,6 +116,8 @@ pub trait Marker {
 /// impl StaticSchemaMarker for TextMarker {
 ///     const UNIQUE_MARKER_TYPE_NAME: &'static str = "Text";
 ///
+///     const CATEGORY: Category<'static> = Category("Other", CategoryColor::Gray);
+///
 ///     const LOCATIONS: MarkerLocations = MarkerLocations::MARKER_CHART.union(MarkerLocations::MARKER_TABLE);
 ///     const CHART_LABEL: Option<&'static str> = Some("{marker.data.text}");
 ///     const TABLE_LABEL: Option<&'static str> = Some("{marker.name} - {marker.data.text}");
@@ -136,10 +133,6 @@ pub trait Marker {
 ///         self.name
 ///     }
 ///
-///     fn category(&self, _profile: &mut Profile) -> CategoryHandle {
-///         CategoryHandle::OTHER
-///     }
-///
 ///     fn string_field_value(&self, _field_index: u32) -> StringHandle {
 ///         self.text
 ///     }
@@ -154,6 +147,9 @@ pub trait StaticSchemaMarker {
     /// [`RuntimeSchemaMarkerSchema::type_name`] of this type's schema.
     const UNIQUE_MARKER_TYPE_NAME: &'static str;
 
+    /// The category of this marker. The marker chart groups marker rows by category.
+    const CATEGORY: Category<'static>;
+
     /// An optional description string. Applies to all markers of this type.
     const DESCRIPTION: Option<&'static str> = None;
 
@@ -204,9 +200,6 @@ pub trait StaticSchemaMarker {
     /// used as `{marker.name}` in the various `label` template strings in the schema.
     fn name(&self, profile: &mut Profile) -> StringHandle;
 
-    /// The category of this marker. The marker chart groups marker rows by category.
-    fn category(&self, profile: &mut Profile) -> CategoryHandle;
-
     /// Called for any fields defined in the schema whose [`format`](RuntimeSchemaMarkerField::format) is
     /// of [kind](MarkerFieldFormat::kind) [`MarkerFieldFormatKind::String`].
     ///
@@ -243,10 +236,6 @@ impl<T: StaticSchemaMarker> Marker for T {
         <T as StaticSchemaMarker>::name(self, profile)
     }
 
-    fn category(&self, profile: &mut Profile) -> CategoryHandle {
-        <T as StaticSchemaMarker>::category(self, profile)
-    }
-
     fn string_field_value(&self, field_index: u32) -> StringHandle {
         <T as StaticSchemaMarker>::string_field_value(self, field_index)
     }
@@ -265,12 +254,14 @@ impl<T: StaticSchemaMarker> Marker for T {
 /// ```
 /// use fxprof_processed_profile::{
 ///     Profile, Marker, MarkerLocations, MarkerFieldFlags, MarkerFieldFormat, RuntimeSchemaMarkerSchema, RuntimeSchemaMarkerField,
-///     CategoryHandle, StringHandle,
+///     CategoryHandle, StringHandle, Category, CategoryColor,
 /// };
 ///
 /// # fn fun() {
+/// # let mut profile = Profile::new("My app", std::time::SystemTime::now().into(), fxprof_processed_profile::SamplingInterval::from_millis(1));
 /// let schema = RuntimeSchemaMarkerSchema {
 ///     type_name: "custom".into(),
+///     category: profile.handle_for_category(Category("Custom", CategoryColor::Purple)),
 ///     locations: MarkerLocations::MARKER_CHART | MarkerLocations::MARKER_TABLE,
 ///     chart_label: Some("{marker.data.eventName}".into()),
 ///     tooltip_label: Some("Custom {marker.name} marker".into()),
@@ -312,6 +303,9 @@ pub struct RuntimeSchemaMarkerSchema {
     /// with the same name.
     pub type_name: String,
 
+    /// The category shared by all markers of this schema.
+    pub category: CategoryHandle,
+
     /// An optional description string. Applies to all markers of this type.
     pub description: Option<String>,
 
@@ -636,6 +630,8 @@ pub struct InternalMarkerSchema {
     /// The name of this marker type.
     type_name: String,
 
+    category: CategoryHandle,
+
     /// List of marker display locations.
     locations: MarkerLocations,
 
@@ -675,6 +671,7 @@ impl InternalMarkerSchema {
             .count();
         Self {
             type_name: schema.type_name,
+            category: schema.category,
             locations: schema.locations,
             chart_label: schema.chart_label,
             tooltip_label: schema.tooltip_label,
@@ -687,7 +684,7 @@ impl InternalMarkerSchema {
         }
     }
 
-    pub fn from_static_schema<T: StaticSchemaMarker>() -> Self {
+    pub fn from_static_schema<T: StaticSchemaMarker>(profile: &mut Profile) -> Self {
         let string_field_count = T::FIELDS
             .iter()
             .filter(|f| f.format.kind() == MarkerFieldFormatKind::String)
@@ -698,6 +695,7 @@ impl InternalMarkerSchema {
             .count();
         Self {
             type_name: T::UNIQUE_MARKER_TYPE_NAME.into(),
+            category: profile.handle_for_category(T::CATEGORY),
             locations: T::LOCATIONS,
             chart_label: T::CHART_LABEL.map(Into::into),
             tooltip_label: T::TOOLTIP_LABEL.map(Into::into),
@@ -713,6 +711,9 @@ impl InternalMarkerSchema {
     pub fn type_name(&self) -> &str {
         &self.type_name
     }
+    pub fn category(&self) -> CategoryHandle {
+        self.category
+    }
     pub fn fields(&self) -> &[RuntimeSchemaMarkerField] {
         &self.fields
     }

fxprof-processed-profile/src/profile.rs

@@ -122,7 +122,7 @@ pub struct Profile {
     pub(crate) global_libs: GlobalLibTable,
     pub(crate) kernel_libs: LibMappings<LibraryHandle>,
     pub(crate) categories: FastIndexSet<InternalCategory>, // append-only for stable CategoryHandles
-    pub(crate) processes: Vec<Process>,                // append-only for stable ProcessHandles
+    pub(crate) processes: Vec<Process>,                    // append-only for stable ProcessHandles
     pub(crate) counters: Vec<Counter>,
     pub(crate) threads: Vec<Thread>, // append-only for stable ThreadHandles
     pub(crate) initial_visible_threads: Vec<ThreadHandle>,
@@ -694,19 +694,19 @@ impl Profile {
     /// You usually don't need to call this, ever. It is called by the blanket impl
     /// of [`Marker::marker_type`] for all types which implement [`StaticSchemaMarker`].
     pub fn static_schema_marker_type<T: StaticSchemaMarker>(&mut self) -> MarkerTypeHandle {
-        match self
+        if let Some(handle) = self
             .static_schema_marker_types
-            .entry(T::UNIQUE_MARKER_TYPE_NAME)
+            .get(T::UNIQUE_MARKER_TYPE_NAME)
         {
-            Entry::Occupied(entry) => *entry.get(),
-            Entry::Vacant(entry) => {
-                let handle = MarkerTypeHandle(self.marker_schemas.len());
-                let schema = InternalMarkerSchema::from_static_schema::<T>();
-                self.marker_schemas.push(schema);
-                entry.insert(handle);
-                handle
-            }
+            return *handle;
         }
+
+        let handle = MarkerTypeHandle(self.marker_schemas.len());
+        let schema = InternalMarkerSchema::from_static_schema::<T>(self);
+        self.marker_schemas.push(schema);
+        self.static_schema_marker_types
+            .insert(T::UNIQUE_MARKER_TYPE_NAME, handle);
+        handle
     }
 
     /// Add a marker to the given thread.
@@ -715,7 +715,7 @@ impl Profile {
     ///
     /// ```
     /// use fxprof_processed_profile::{
-    ///     Profile, CategoryHandle, Marker, MarkerFieldFlags, MarkerFieldFormat, MarkerTiming,
+    ///     Profile, Category, CategoryColor, Marker, MarkerFieldFlags, MarkerFieldFormat, MarkerTiming,
     ///     StaticSchemaMarker, StaticSchemaMarkerField, StringHandle, ThreadHandle, Timestamp,
     /// };
     ///
@@ -739,6 +739,8 @@ impl Profile {
     /// impl StaticSchemaMarker for TextMarker {
     ///     const UNIQUE_MARKER_TYPE_NAME: &'static str = "Text";
     ///
+    ///     const CATEGORY: Category<'static> = Category("Other", CategoryColor::Gray);
+    ///
     ///     const CHART_LABEL: Option<&'static str> = Some("{marker.data.text}");
     ///     const TABLE_LABEL: Option<&'static str> = Some("{marker.name} - {marker.data.text}");
     ///
@@ -753,10 +755,6 @@ impl Profile {
     ///         self.name
     ///     }
     ///
-    ///     fn category(&self, _profile: &mut Profile) -> CategoryHandle {
-    ///         CategoryHandle::OTHER
-    ///     }
-    ///
     ///     fn string_field_value(&self, _field_index: u32) -> StringHandle {
     ///         self.text
     ///     }
@@ -774,7 +772,6 @@ impl Profile {
     ) -> MarkerHandle {
         let marker_type = marker.marker_type(self);
         let name = marker.name(self);
-        let category = marker.category(self);
         let thread = &mut self.threads[thread.0];
         let name_thread_string_index = thread.convert_string_index(&self.string_table, name.0);
         let schema = &self.marker_schemas[marker_type.0];
@@ -784,7 +781,6 @@ impl Profile {
             schema,
             marker,
             timing,
-            category,
             &mut self.string_table,
         )
     }

fxprof-processed-profile/src/thread.rs

@@ -15,7 +15,7 @@ use crate::sample_table::{NativeAllocationsTable, SampleTable, WeightType};
 use crate::stack_table::StackTable;
 use crate::string_table::{GlobalStringIndex, GlobalStringTable};
 use crate::thread_string_table::{ThreadInternalStringIndex, ThreadStringTable};
-use crate::{CategoryHandle, Marker, MarkerHandle, MarkerTiming, MarkerTypeHandle, Timestamp};
+use crate::{Marker, MarkerHandle, MarkerTiming, MarkerTypeHandle, Timestamp};
 
 /// A process. Can be created with [`Profile::add_process`](crate::Profile::add_process).
 #[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)]
@@ -169,7 +169,6 @@ impl Thread {
         schema: &InternalMarkerSchema,
         marker: T,
         timing: MarkerTiming,
-        category: CategoryHandle,
         global_string_table: &mut GlobalStringTable,
     ) -> MarkerHandle {
         self.markers.add_marker(
@@ -178,7 +177,6 @@ impl Thread {
             schema,
             marker,
             timing,
-            category,
             &mut self.string_table,
             global_string_table,
         )

fxprof-processed-profile/tests/integration_tests/main.rs

@@ -4,8 +4,8 @@ use std::time::Duration;
 use assert_json_diff::assert_json_eq;
 use debugid::DebugId;
 use fxprof_processed_profile::{
-    Category, CategoryColor, CategoryHandle, CpuDelta, Frame, FrameFlags, FrameInfo, GraphColor,
-    LibraryInfo, MarkerFieldFlags, MarkerFieldFormat, MarkerGraphType, MarkerTiming, Profile,
+    Category, CategoryColor, CpuDelta, Frame, FrameFlags, FrameInfo, GraphColor, LibraryInfo,
+    MarkerFieldFlags, MarkerFieldFormat, MarkerGraphType, MarkerTiming, Profile,
     ReferenceTimestamp, SamplingInterval, StaticSchemaMarker, StaticSchemaMarkerField,
     StaticSchemaMarkerGraph, StringHandle, Symbol, SymbolTable, Timestamp, WeightType,
 };
@@ -22,6 +22,7 @@ pub struct TextMarker {
 
 impl StaticSchemaMarker for TextMarker {
     const UNIQUE_MARKER_TYPE_NAME: &'static str = "Text";
+    const CATEGORY: Category<'static> = Category("Other", CategoryColor::Gray);
     const CHART_LABEL: Option<&'static str> = Some("{marker.data.name}");
     const TABLE_LABEL: Option<&'static str> = Some("{marker.name} - {marker.data.name}");
     const FIELDS: &'static [StaticSchemaMarkerField] = &[StaticSchemaMarkerField {
@@ -35,10 +36,6 @@ impl StaticSchemaMarker for TextMarker {
         self.name
     }
 
-    fn category(&self, _profile: &mut Profile) -> CategoryHandle {
-        CategoryHandle::OTHER
-    }
-
     fn string_field_value(&self, _field_index: u32) -> StringHandle {
         self.text
     }
@@ -58,6 +55,7 @@ fn profile_without_js() {
     }
     impl StaticSchemaMarker for CustomMarker {
         const UNIQUE_MARKER_TYPE_NAME: &'static str = "custom";
+        const CATEGORY: Category<'static> = Category("Other", CategoryColor::Gray);
         const TOOLTIP_LABEL: Option<&'static str> = Some("Custom tooltip label");
 
         const FIELDS: &'static [StaticSchemaMarkerField] = &[
@@ -100,10 +98,6 @@ fn profile_without_js() {
             profile.handle_for_string("CustomName")
         }
 
-        fn category(&self, _profile: &mut Profile) -> CategoryHandle {
-            CategoryHandle::OTHER
-        }
-
         fn string_field_value(&self, field_index: u32) -> StringHandle {
             match field_index {
                 0 => self.event_name,

samply/src/linux_shared/converter.rs

@@ -7,10 +7,10 @@ use byteorder::LittleEndian;
 use debugid::DebugId;
 use framehop::{ExplicitModuleSectionInfo, FrameAddress, Module, Unwinder};
 use fxprof_processed_profile::{
-    Category, CategoryColor, CategoryHandle, CpuDelta, LibraryHandle, LibraryInfo,
-    MarkerFieldFlags, MarkerFieldFormat, MarkerTiming, Profile, ReferenceTimestamp,
-    SamplingInterval, StaticSchemaMarker, StaticSchemaMarkerField, StringHandle, SubcategoryHandle,
-    SymbolTable, ThreadHandle,
+    Category, CategoryColor, CpuDelta, LibraryHandle, LibraryInfo, MarkerFieldFlags,
+    MarkerFieldFormat, MarkerTiming, Profile, ReferenceTimestamp, SamplingInterval,
+    StaticSchemaMarker, StaticSchemaMarkerField, StringHandle, SubcategoryHandle, SymbolTable,
+    ThreadHandle,
 };
 use linux_perf_data::linux_perf_event_reader::TaskWasPreempted;
 use linux_perf_data::simpleperf_dso_type::{DSO_DEX_FILE, DSO_KERNEL, DSO_KERNEL_MODULE};
@@ -1905,6 +1905,8 @@ struct MmapMarker(StringHandle);
 impl StaticSchemaMarker for MmapMarker {
     const UNIQUE_MARKER_TYPE_NAME: &'static str = "mmap";
 
+    const CATEGORY: Category<'static> = Category("Other", CategoryColor::Gray);
+
     const FIELDS: &'static [StaticSchemaMarkerField] = &[StaticSchemaMarkerField {
         key: "name",
         label: "Details",
@@ -1916,10 +1918,6 @@ impl StaticSchemaMarker for MmapMarker {
         profile.handle_for_string("mmap")
     }
 
-    fn category(&self, _profile: &mut Profile) -> CategoryHandle {
-        CategoryHandle::OTHER
-    }
-
     fn string_field_value(&self, _field_index: u32) -> StringHandle {
         self.0
     }