Skip to content

Commit 229c9e8

Browse files
mstangemstange
authored
Add support for Flow related marker field formats to fxprof-processed-profile (#613)
2 parents 0ec0ddb + 89c031a commit 229c9e8

File tree

11 files changed

+434
-7
lines changed

11 files changed

+434
-7
lines changed

‎fxprof-processed-profile/src/marker_table.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ pub struct MarkerTable {
3737
/// https://github.com/firefox-devtools/profiler/issues/5022 tracks supporting string indexes
3838
/// for the other string format variants.
3939
marker_field_string_values: Vec<StringHandle>,
40+
/// The field values for any marker fields of [kind](`MarkerFieldFormat::kind`) [`MarkerFieldFormatKind::Flow`].
41+
///
42+
/// This Vec can contain zero or more values per marker, depending on the marker's
43+
/// type's schema's `flow_field_count`. For the marker with index i,
44+
/// its field values will be at index sum_{k in 0..i}(marker_schema[k].flow_field_count).
45+
///
46+
/// Flow identifiers are serialized as string indexes.
47+
marker_field_flow_values: Vec<StringHandle>,
4048
}
4149

4250
impl MarkerTable {
@@ -47,6 +55,7 @@ impl MarkerTable {
4755
#[allow(clippy::too_many_arguments)]
4856
pub fn add_marker<T: Marker>(
4957
&mut self,
58+
string_table: &mut ProfileStringTable,
5059
name_string_index: StringHandle,
5160
marker_type_handle: MarkerTypeHandle,
5261
schema: &InternalMarkerSchema,
@@ -76,6 +85,13 @@ impl MarkerTable {
7685
let number_value = marker.number_field_value(field_index as u32);
7786
self.marker_field_number_values.push(number_value)
7887
}
88+
MarkerFieldFormatKind::Flow => {
89+
let flow_value = marker.flow_field_value(field_index as u32);
90+
// Convert flow ID to hex string and store as StringHandle
91+
let hex_string = format!("{flow_value:x}");
92+
let flow_string_handle = string_table.index_for_string(&hex_string);
93+
self.marker_field_flow_values.push(flow_string_handle);
94+
}
7995
}
8096
}
8197

@@ -138,22 +154,27 @@ impl Serialize for SerializableMarkerTableDataColumn<'_> {
138154
let mut seq = serializer.serialize_seq(Some(len))?;
139155
let mut remaining_string_fields = &marker_table.marker_field_string_values[..];
140156
let mut remaining_number_fields = &marker_table.marker_field_number_values[..];
157+
let mut remaining_flow_fields = &marker_table.marker_field_flow_values[..];
141158
for i in 0..len {
142159
let marker_type_handle = marker_table.marker_type_handles[i];
143160
let stack_index = marker_table.marker_stacks[i];
144161
let schema = &schemas[marker_type_handle.0];
145162
let string_fields;
146163
let number_fields;
164+
let flow_fields;
147165
(string_fields, remaining_string_fields) =
148166
remaining_string_fields.split_at(schema.string_field_count());
149167
(number_fields, remaining_number_fields) =
150168
remaining_number_fields.split_at(schema.number_field_count());
169+
(flow_fields, remaining_flow_fields) =
170+
remaining_flow_fields.split_at(schema.flow_field_count());
151171
seq.serialize_element(&SerializableMarkerDataElement {
152172
string_table,
153173
stack_index,
154174
schema,
155175
string_fields,
156176
number_fields,
177+
flow_fields,
157178
})?;
158179
}
159180
seq.end()
@@ -166,6 +187,7 @@ struct SerializableMarkerDataElement<'a> {
166187
schema: &'a InternalMarkerSchema,
167188
string_fields: &'a [StringHandle],
168189
number_fields: &'a [f64],
190+
flow_fields: &'a [StringHandle],
169191
}
170192

171193
impl Serialize for SerializableMarkerDataElement<'_> {
@@ -176,6 +198,7 @@ impl Serialize for SerializableMarkerDataElement<'_> {
176198
schema,
177199
mut string_fields,
178200
mut number_fields,
201+
mut flow_fields,
179202
} = self;
180203
let mut map = serializer.serialize_map(None)?;
181204
map.serialize_entry("type", &schema.type_name())?;
@@ -199,6 +222,11 @@ impl Serialize for SerializableMarkerDataElement<'_> {
199222
(value, number_fields) = number_fields.split_first().unwrap();
200223
map.serialize_entry(&field.key, value)?;
201224
}
225+
MarkerFieldFormatKind::Flow => {
226+
let value;
227+
(value, flow_fields) = flow_fields.split_first().unwrap();
228+
map.serialize_entry(&field.key, value)?;
229+
}
202230
}
203231
}
204232

‎fxprof-processed-profile/src/markers.rs

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,21 @@ pub trait Marker {
9090
/// If you do see unexpected calls to this method, make sure you're not registering
9191
/// multiple different schemas with the same [`RuntimeSchemaMarkerSchema::type_name`].
9292
fn number_field_value(&self, field_index: u32) -> f64;
93+
94+
/// Called for any fields defined in the schema whose [`format`](RuntimeSchemaMarkerField::format) is
95+
/// of [kind](MarkerFieldFormat::kind) [`MarkerFieldFormatKind::Flow`].
96+
///
97+
/// `field_index` is an index into the schema's [`fields`](RuntimeSchemaMarkerSchema::fields).
98+
///
99+
/// Flow identifiers are u64 values that are unique across processes.
100+
///
101+
/// You can panic for any unexpected field indexes, for example
102+
/// using `unreachable!()`. You can even panic unconditionally if this
103+
/// marker type doesn't have any flow fields.
104+
///
105+
/// If you do see unexpected calls to this method, make sure you're not registering
106+
/// multiple different schemas with the same [`RuntimeSchemaMarkerSchema::type_name`].
107+
fn flow_field_value(&self, field_index: u32) -> u64;
93108
}
94109

95110
/// The trait for markers whose schema is known at compile time. Any type which implements
@@ -140,6 +155,10 @@ pub trait Marker {
140155
/// fn number_field_value(&self, _field_index: u32) -> f64 {
141156
/// unreachable!()
142157
/// }
158+
///
159+
/// fn flow_field_value(&self, _field_index: u32) -> u64 {
160+
/// unreachable!()
161+
/// }
143162
/// }
144163
/// ```
145164
pub trait StaticSchemaMarker {
@@ -225,6 +244,21 @@ pub trait StaticSchemaMarker {
225244
/// If you do see unexpected calls to this method, make sure you're not registering
226245
/// multiple different schemas with the same [`RuntimeSchemaMarkerSchema::type_name`].
227246
fn number_field_value(&self, field_index: u32) -> f64;
247+
248+
/// Called for any fields defined in the schema whose [`format`](RuntimeSchemaMarkerField::format) is
249+
/// of [kind](MarkerFieldFormat::kind) [`MarkerFieldFormatKind::Flow`].
250+
///
251+
/// `field_index` is an index into the schema's [`fields`](RuntimeSchemaMarkerSchema::fields).
252+
///
253+
/// Flow identifiers are u64 values that are unique across processes.
254+
///
255+
/// You can panic for any unexpected field indexes, for example
256+
/// using `unreachable!()`. You can even panic unconditionally if this
257+
/// marker type doesn't have any flow fields.
258+
///
259+
/// If you do see unexpected calls to this method, make sure you're not registering
260+
/// multiple different schemas with the same [`RuntimeSchemaMarkerSchema::type_name`].
261+
fn flow_field_value(&self, field_index: u32) -> u64;
228262
}
229263

230264
impl<T: StaticSchemaMarker> Marker for T {
@@ -243,6 +277,10 @@ impl<T: StaticSchemaMarker> Marker for T {
243277
fn number_field_value(&self, field_index: u32) -> f64 {
244278
<T as StaticSchemaMarker>::number_field_value(self, field_index)
245279
}
280+
281+
fn flow_field_value(&self, field_index: u32) -> u64 {
282+
<T as StaticSchemaMarker>::flow_field_value(self, field_index)
283+
}
246284
}
247285

248286
/// Describes a marker type, including the names and types of the marker's fields.
@@ -510,20 +548,33 @@ pub enum MarkerFieldFormat {
510548
///
511549
/// "Label: 52.23, 0.0054, 123,456.78"
512550
Decimal,
551+
552+
/// A flow is a u64 identifier that's unique across processes. All of
553+
/// the markers with same flow id before a terminating flow id will be
554+
/// considered part of the same "flow" and linked together.
555+
#[serde(rename = "flow-id")]
556+
Flow,
557+
558+
/// A terminating flow ends a flow of a particular id and allows that id
559+
/// to be reused again. It often makes sense for destructors to create
560+
/// a marker with a field of this type.
561+
#[serde(rename = "terminating-flow-id")]
562+
TerminatingFlow,
513563
}
514564

515-
/// The kind of a marker field. Every marker field is either a string or a number.
565+
/// The kind of a marker field. Every marker field is either a string, a number, or a flow.
516566
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
517567
pub enum MarkerFieldFormatKind {
518568
String,
519569
Number,
570+
Flow,
520571
}
521572

522573
impl MarkerFieldFormat {
523-
/// Whether this field is a number field or a string field.
574+
/// Whether this field is a number field, a string field, or a flow field.
524575
///
525-
/// This determines whether we call `number_field_value` or
526-
/// `string_field_value` to get the field values.
576+
/// This determines whether we call `number_field_value`, `string_field_value`,
577+
/// or `flow_field_value` to get the field values.
527578
pub fn kind(&self) -> MarkerFieldFormatKind {
528579
match self {
529580
Self::Url | Self::FilePath | Self::SanitizedString | Self::String => {
@@ -539,6 +590,7 @@ impl MarkerFieldFormat {
539590
| Self::Percentage
540591
| Self::Integer
541592
| Self::Decimal => MarkerFieldFormatKind::Number,
593+
Self::Flow | Self::TerminatingFlow => MarkerFieldFormatKind::Flow,
542594
}
543595
}
544596
}
@@ -647,6 +699,7 @@ pub struct InternalMarkerSchema {
647699

648700
string_field_count: usize,
649701
number_field_count: usize,
702+
flow_field_count: usize,
650703

651704
description: Option<String>,
652705
}
@@ -669,6 +722,11 @@ impl InternalMarkerSchema {
669722
.iter()
670723
.filter(|f| f.format.kind() == MarkerFieldFormatKind::Number)
671724
.count();
725+
let flow_field_count = schema
726+
.fields
727+
.iter()
728+
.filter(|f| f.format.kind() == MarkerFieldFormatKind::Flow)
729+
.count();
672730
Self {
673731
type_name: schema.type_name,
674732
category: schema.category,
@@ -680,6 +738,7 @@ impl InternalMarkerSchema {
680738
graphs: schema.graphs,
681739
string_field_count,
682740
number_field_count,
741+
flow_field_count,
683742
description: schema.description,
684743
}
685744
}
@@ -693,6 +752,10 @@ impl InternalMarkerSchema {
693752
.iter()
694753
.filter(|f| f.format.kind() == MarkerFieldFormatKind::Number)
695754
.count();
755+
let flow_field_count = T::FIELDS
756+
.iter()
757+
.filter(|f| f.format.kind() == MarkerFieldFormatKind::Flow)
758+
.count();
696759
Self {
697760
type_name: T::UNIQUE_MARKER_TYPE_NAME.into(),
698761
category: profile.handle_for_category(T::CATEGORY),
@@ -703,6 +766,7 @@ impl InternalMarkerSchema {
703766
fields: T::FIELDS.iter().map(Into::into).collect(),
704767
string_field_count,
705768
number_field_count,
769+
flow_field_count,
706770
description: T::DESCRIPTION.map(Into::into),
707771
graphs: T::GRAPHS.iter().map(Into::into).collect(),
708772
}
@@ -723,6 +787,9 @@ impl InternalMarkerSchema {
723787
pub fn number_field_count(&self) -> usize {
724788
self.number_field_count
725789
}
790+
pub fn flow_field_count(&self) -> usize {
791+
self.flow_field_count
792+
}
726793
fn serialize_self<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
727794
where
728795
S: serde::Serializer,

‎fxprof-processed-profile/src/profile.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,10 @@ impl Profile {
10781078
/// fn number_field_value(&self, _field_index: u32) -> f64 {
10791079
/// unreachable!()
10801080
/// }
1081+
///
1082+
/// fn flow_field_value(&self, _field_index: u32) -> u64 {
1083+
/// unreachable!()
1084+
/// }
10811085
/// }
10821086
/// ```
10831087
pub fn add_marker<T: Marker>(
@@ -1088,9 +1092,16 @@ impl Profile {
10881092
) -> MarkerHandle {
10891093
let marker_type = marker.marker_type(self);
10901094
let name = marker.name(self);
1091-
let thread = &mut self.threads[thread.0];
10921095
let schema = &self.marker_schemas[marker_type.0];
1093-
thread.add_marker(name, marker_type, schema, marker, timing)
1096+
let thread_obj = &mut self.threads[thread.0];
1097+
thread_obj.add_marker(
1098+
&mut self.string_table,
1099+
name,
1100+
marker_type,
1101+
schema,
1102+
marker,
1103+
timing,
1104+
)
10941105
}
10951106

10961107
/// Sets a marker's stack. Every marker can have an optional stack, regardless

‎fxprof-processed-profile/src/thread.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,15 @@ impl Thread {
167167
#[allow(clippy::too_many_arguments)]
168168
pub fn add_marker<T: Marker>(
169169
&mut self,
170+
string_table: &mut ProfileStringTable,
170171
name_string_index: StringHandle,
171172
marker_type_handle: MarkerTypeHandle,
172173
schema: &InternalMarkerSchema,
173174
marker: T,
174175
timing: MarkerTiming,
175176
) -> MarkerHandle {
176177
self.markers.add_marker(
178+
string_table,
177179
name_string_index,
178180
marker_type_handle,
179181
schema,

0 commit comments

Comments
 (0)