Skip to content

Commit

Permalink
Make StaticSchemaMarker interface more ergonomic.
Browse files Browse the repository at this point in the history
  • Loading branch information
mstange committed Feb 1, 2025
1 parent 7fa4115 commit a757ecd
Show file tree
Hide file tree
Showing 10 changed files with 688 additions and 657 deletions.
7 changes: 4 additions & 3 deletions fxprof-processed-profile/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ pub use global_lib_table::{LibraryHandle, UsedLibraryAddressesIterator};
pub use lib_mappings::LibMappings;
pub use library_info::{LibraryInfo, Symbol, SymbolTable};
pub use markers::{
GraphColor, Marker, MarkerFieldFormat, MarkerFieldFormatKind, MarkerFieldSchema,
MarkerGraphSchema, MarkerGraphType, MarkerHandle, MarkerLocation, MarkerSchema,
MarkerStaticField, MarkerTiming, MarkerTypeHandle, StaticSchemaMarker,
GraphColor, Marker, MarkerFieldFlags, MarkerFieldFormat, MarkerFieldFormatKind,
MarkerGraphType, MarkerHandle, MarkerLocations, MarkerTiming, MarkerTypeHandle,
RuntimeSchemaMarkerField, RuntimeSchemaMarkerGraph, RuntimeSchemaMarkerSchema,
StaticSchemaMarker, StaticSchemaMarkerField, StaticSchemaMarkerGraph,
};
pub use process::ThreadHandle;
pub use profile::{FrameHandle, Profile, SamplingInterval, StackHandle, StringHandle};
Expand Down
660 changes: 424 additions & 236 deletions fxprof-processed-profile/src/markers.rs

Large diffs are not rendered by default.

46 changes: 19 additions & 27 deletions fxprof-processed-profile/src/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use crate::global_lib_table::{GlobalLibTable, LibraryHandle, UsedLibraryAddresse
use crate::lib_mappings::LibMappings;
use crate::library_info::{LibraryInfo, SymbolTable};
use crate::markers::{
GraphColor, InternalMarkerSchema, Marker, MarkerHandle, MarkerSchema, MarkerTiming,
MarkerTypeHandle, StaticSchemaMarker,
GraphColor, InternalMarkerSchema, Marker, MarkerHandle, MarkerTiming, MarkerTypeHandle,
RuntimeSchemaMarkerSchema, StaticSchemaMarker,
};
use crate::process::{Process, ThreadHandle};
use crate::reference_timestamp::ReferenceTimestamp;
Expand Down Expand Up @@ -423,7 +423,7 @@ impl Profile {
self.threads[thread.0].set_tid(tid);
}

/// Set whether to show a timeline view displaying [`MarkerLocation::TimelineOverview`](crate::MarkerLocation::TimelineOverview)
/// Set whether to show a timeline which displays [`MarkerLocations::TIMELINE_OVERVIEW`](crate::MarkerLocations::TIMELINE_OVERVIEW)
/// markers for this thread.
///
/// Main threads always have such a timeline view and always display such markers,
Expand Down Expand Up @@ -670,16 +670,16 @@ impl Profile {
);
}

/// Registers a marker type, given the type's [`MarkerSchema`]. Usually you only need to call this for
/// Registers a marker type for a [`RuntimeSchemaMarkerSchema`]. You only need to call this for
/// marker types whose schema is dynamically created at runtime.
///
/// After you register the marker type, you'll save its [`MarkerTypeHandle`] somewhere, and then
/// store it in every marker you create of this type. The marker then needs to return the
/// handle from its implementation of [`Marker::marker_type`].
///
/// For marker types whose schema is known at compile time, you'll want to implement
/// [`StaticSchemaMarker`] instead.
pub fn register_marker_type(&mut self, schema: MarkerSchema) -> MarkerTypeHandle {
/// [`StaticSchemaMarker`] instead, and you don't need to call this method.
pub fn register_marker_type(&mut self, schema: RuntimeSchemaMarkerSchema) -> MarkerTypeHandle {
let handle = MarkerTypeHandle(self.marker_schemas.len());
self.marker_schemas.push(schema.into());
handle
Expand All @@ -697,7 +697,8 @@ impl Profile {
Entry::Occupied(entry) => *entry.get(),
Entry::Vacant(entry) => {
let handle = MarkerTypeHandle(self.marker_schemas.len());
self.marker_schemas.push(T::schema().into());
let schema = InternalMarkerSchema::from_static_schema::<T>();
self.marker_schemas.push(schema);
entry.insert(handle);
handle
}
Expand All @@ -710,9 +711,8 @@ impl Profile {
///
/// ```
/// use fxprof_processed_profile::{
/// Profile, Marker, MarkerTiming, MarkerLocation, MarkerFieldFormat, MarkerSchema,
/// MarkerFieldSchema, StaticSchemaMarker, CategoryHandle, StringHandle, ThreadHandle,
/// Timestamp,
/// Profile, CategoryHandle, Marker, MarkerFieldFlags, MarkerFieldFormat, MarkerTiming,
/// StaticSchemaMarker, StaticSchemaMarkerField, StringHandle, ThreadHandle, Timestamp,
/// };
///
/// # fn fun() {
Expand All @@ -735,23 +735,15 @@ impl Profile {
/// impl StaticSchemaMarker for TextMarker {
/// const UNIQUE_MARKER_TYPE_NAME: &'static str = "Text";
///
/// fn schema() -> MarkerSchema {
/// MarkerSchema {
/// type_name: Self::UNIQUE_MARKER_TYPE_NAME.into(),
/// locations: vec![MarkerLocation::MarkerChart, MarkerLocation::MarkerTable],
/// chart_label: Some("{marker.data.text}".into()),
/// tooltip_label: None,
/// table_label: Some("{marker.name} - {marker.data.text}".into()),
/// fields: vec![MarkerFieldSchema {
/// key: "text".into(),
/// label: "Contents".into(),
/// format: MarkerFieldFormat::String,
/// searchable: true,
/// }],
/// static_fields: vec![],
/// graphs: vec![],
/// }
/// }
/// const CHART_LABEL: Option<&'static str> = Some("{marker.data.text}");
/// const TABLE_LABEL: Option<&'static str> = Some("{marker.name} - {marker.data.text}");
///
/// const FIELDS: &'static [StaticSchemaMarkerField] = &[StaticSchemaMarkerField {
/// key: "text",
/// label: "Contents",
/// format: MarkerFieldFormat::String,
/// flags: MarkerFieldFlags::SEARCHABLE,
/// }];
///
/// fn name(&self, _profile: &mut Profile) -> StringHandle {
/// self.name
Expand Down
111 changes: 46 additions & 65 deletions fxprof-processed-profile/tests/integration_tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use assert_json_diff::assert_json_eq;
use debugid::DebugId;
use fxprof_processed_profile::{
CategoryColor, CategoryHandle, CpuDelta, Frame, FrameFlags, FrameInfo, GraphColor, LibraryInfo,
MarkerFieldFormat, MarkerFieldSchema, MarkerGraphSchema, MarkerGraphType, MarkerLocation,
MarkerSchema, MarkerStaticField, MarkerTiming, Profile, ReferenceTimestamp, SamplingInterval,
StaticSchemaMarker, StringHandle, Symbol, SymbolTable, Timestamp, WeightType,
MarkerFieldFlags, MarkerFieldFormat, MarkerGraphType, MarkerTiming, Profile,
ReferenceTimestamp, SamplingInterval, StaticSchemaMarker, StaticSchemaMarkerField,
StaticSchemaMarkerGraph, StringHandle, Symbol, SymbolTable, Timestamp, WeightType,
};
use serde_json::json;

Expand All @@ -22,24 +22,14 @@ pub struct TextMarker {

impl StaticSchemaMarker for TextMarker {
const UNIQUE_MARKER_TYPE_NAME: &'static str = "Text";

fn schema() -> MarkerSchema {
MarkerSchema {
type_name: Self::UNIQUE_MARKER_TYPE_NAME.into(),
locations: vec![MarkerLocation::MarkerChart, MarkerLocation::MarkerTable],
chart_label: Some("{marker.data.name}".into()),
tooltip_label: None,
table_label: Some("{marker.name} - {marker.data.name}".into()),
fields: vec![MarkerFieldSchema {
key: "name".into(),
label: "Details".into(),
format: MarkerFieldFormat::String,
searchable: true,
}],
static_fields: vec![],
graphs: vec![],
}
}
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 {
key: "name",
label: "Details",
format: MarkerFieldFormat::String,
flags: MarkerFieldFlags::SEARCHABLE,
}];

fn name(&self, _profile: &mut Profile) -> StringHandle {
self.name
Expand Down Expand Up @@ -68,52 +58,43 @@ fn profile_without_js() {
}
impl StaticSchemaMarker for CustomMarker {
const UNIQUE_MARKER_TYPE_NAME: &'static str = "custom";
const TOOLTIP_LABEL: Option<&'static str> = Some("Custom tooltip label");

fn schema() -> MarkerSchema {
MarkerSchema {
type_name: Self::UNIQUE_MARKER_TYPE_NAME.into(),
locations: vec![MarkerLocation::MarkerChart, MarkerLocation::MarkerTable],
chart_label: None,
tooltip_label: Some("Custom tooltip label".into()),
table_label: None,
fields: vec![
MarkerFieldSchema {
key: "eventName".into(),
label: "Event name".into(),
format: MarkerFieldFormat::String,
searchable: true,
},
MarkerFieldSchema {
key: "allocationSize".into(),
label: "Allocation size".into(),
format: MarkerFieldFormat::Bytes,
searchable: true,
},
MarkerFieldSchema {
key: "url".into(),
label: "URL".into(),
format: MarkerFieldFormat::Url,
searchable: true,
},
MarkerFieldSchema {
key: "latency".into(),
label: "Latency".into(),
format: MarkerFieldFormat::Duration,
searchable: true,
},
],
static_fields: vec![MarkerStaticField {
label: "Description".into(),
value: "This is a test marker with a custom schema.".into(),
}],
const FIELDS: &'static [StaticSchemaMarkerField] = &[
StaticSchemaMarkerField {
key: "eventName",
label: "Event name",
format: MarkerFieldFormat::String,
flags: MarkerFieldFlags::SEARCHABLE,
},
StaticSchemaMarkerField {
key: "allocationSize",
label: "Allocation size",
format: MarkerFieldFormat::Bytes,
flags: MarkerFieldFlags::SEARCHABLE,
},
StaticSchemaMarkerField {
key: "url",
label: "URL",
format: MarkerFieldFormat::Url,
flags: MarkerFieldFlags::SEARCHABLE,
},
StaticSchemaMarkerField {
key: "latency",
label: "Latency",
format: MarkerFieldFormat::Duration,
flags: MarkerFieldFlags::SEARCHABLE,
},
];

graphs: vec![MarkerGraphSchema {
key: "latency",
graph_type: MarkerGraphType::Line,
color: Some(GraphColor::Green),
}],
}
}
const DESCRIPTION: Option<&'static str> =
Some("This is a test marker with a custom schema.");

const GRAPHS: &'static [StaticSchemaMarkerGraph] = &[StaticSchemaMarkerGraph {
key: "latency",
graph_type: MarkerGraphType::Line,
color: Some(GraphColor::Green),
}];

fn name(&self, profile: &mut Profile) -> StringHandle {
profile.intern_string("CustomName")
Expand Down
28 changes: 9 additions & 19 deletions samply/src/linux_shared/converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use debugid::DebugId;
use framehop::{ExplicitModuleSectionInfo, FrameAddress, Module, Unwinder};
use fxprof_processed_profile::{
CategoryColor, CategoryHandle, CategoryPairHandle, CpuDelta, LibraryHandle, LibraryInfo,
MarkerFieldFormat, MarkerFieldSchema, MarkerLocation, MarkerSchema, MarkerTiming, Profile,
ReferenceTimestamp, SamplingInterval, StaticSchemaMarker, StringHandle, SymbolTable,
MarkerFieldFlags, MarkerFieldFormat, MarkerTiming, Profile, ReferenceTimestamp,
SamplingInterval, StaticSchemaMarker, StaticSchemaMarkerField, StringHandle, SymbolTable,
ThreadHandle,
};
use linux_perf_data::linux_perf_event_reader::TaskWasPreempted;
Expand Down Expand Up @@ -1902,23 +1902,13 @@ struct MmapMarker(StringHandle);

impl StaticSchemaMarker for MmapMarker {
const UNIQUE_MARKER_TYPE_NAME: &'static str = "mmap";
fn schema() -> MarkerSchema {
MarkerSchema {
type_name: Self::UNIQUE_MARKER_TYPE_NAME.into(),
locations: vec![MarkerLocation::MarkerChart, MarkerLocation::MarkerTable],
chart_label: Some("{marker.data.name}".into()),
tooltip_label: Some("{marker.name} - {marker.data.name}".into()),
table_label: Some("{marker.name} - {marker.data.name}".into()),
fields: vec![MarkerFieldSchema {
key: "name".into(),
label: "Details".into(),
format: MarkerFieldFormat::String,
searchable: true,
}],
static_fields: vec![],
graphs: vec![],
}
}

const FIELDS: &'static [StaticSchemaMarkerField] = &[StaticSchemaMarkerField {
key: "name",
label: "Details",
format: MarkerFieldFormat::String,
flags: MarkerFieldFlags::SEARCHABLE,
}];

fn name(&self, profile: &mut Profile) -> StringHandle {
profile.intern_string("mmap")
Expand Down
37 changes: 15 additions & 22 deletions samply/src/shared/jit_function_add_marker.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use fxprof_processed_profile::{
CategoryHandle, MarkerFieldFormat, MarkerFieldSchema, MarkerLocation, MarkerSchema,
MarkerStaticField, Profile, StaticSchemaMarker, StringHandle,
CategoryHandle, MarkerFieldFlags, MarkerFieldFormat, Profile, StaticSchemaMarker,
StaticSchemaMarkerField, StringHandle,
};

#[derive(Debug, Clone)]
Expand All @@ -9,26 +9,19 @@ pub struct JitFunctionAddMarker(pub StringHandle);
impl StaticSchemaMarker for JitFunctionAddMarker {
const UNIQUE_MARKER_TYPE_NAME: &'static str = "JitFunctionAdd";

fn schema() -> MarkerSchema {
MarkerSchema {
type_name: Self::UNIQUE_MARKER_TYPE_NAME.into(),
locations: vec![MarkerLocation::MarkerChart, MarkerLocation::MarkerTable],
chart_label: Some("{marker.data.n}".into()),
tooltip_label: Some("{marker.data.n}".into()),
table_label: Some("{marker.data.n}".into()),
fields: vec![MarkerFieldSchema {
key: "n".into(),
label: "Function".into(),
format: MarkerFieldFormat::String,
searchable: true,
}],
static_fields: vec![MarkerStaticField {
label: "Description".into(),
value: "Emitted when a JIT function is added to the process.".into(),
}],
graphs: vec![],
}
}
const DESCRIPTION: Option<&'static str> =
Some("Emitted when a JIT function is added to the process.");

const CHART_LABEL: Option<&'static str> = Some("{marker.data.n}");
const TOOLTIP_LABEL: Option<&'static str> = Some("{marker.data.n}");
const TABLE_LABEL: Option<&'static str> = Some("{marker.data.n}");

const FIELDS: &'static [StaticSchemaMarkerField] = &[StaticSchemaMarkerField {
key: "n",
label: "Function",
format: MarkerFieldFormat::String,
flags: MarkerFieldFlags::SEARCHABLE,
}];

fn name(&self, profile: &mut Profile) -> StringHandle {
profile.intern_string("JitFunctionAdd")
Expand Down
Loading

0 comments on commit a757ecd

Please sign in to comment.