Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make StaticSchemaMarker interface more ergonomic. #479

Merged
merged 1 commit into from
Feb 1, 2025
Merged

Make StaticSchemaMarker interface more ergonomic. #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions fxprof-processed-profile/src/lib.rs
View file
Open in desktop
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
View file
Open in desktop

Large diffs are not rendered by default.

46 changes: 19 additions & 27 deletions fxprof-processed-profile/src/profile.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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