Skip to content

Commit d47b3cd

Browse files
authored
Improve the markers API in fxprof-processed-profile (#712)
See the diff of `fxprof-processed-profile/tests/integration_tests/main.rs` for the before/after.
2 parents 2041b95 + 632105c commit d47b3cd

File tree

20 files changed

+1385
-1354
lines changed

20 files changed

+1385
-1354
lines changed

fxprof-processed-profile/src/lib.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,11 @@ pub use global_lib_table::LibraryHandle;
8181
pub use lib_mappings::LibMappings;
8282
pub use library_info::{LibraryInfo, Symbol, SymbolTable};
8383
pub use markers::{
84-
GraphColor, Marker, MarkerFieldFlags, MarkerFieldFormat, MarkerFieldFormatKind,
85-
MarkerGraphType, MarkerHandle, MarkerLocations, MarkerTiming, MarkerTypeHandle,
86-
RuntimeSchemaMarkerField, RuntimeSchemaMarkerGraph, RuntimeSchemaMarkerSchema,
87-
StaticSchemaMarker, StaticSchemaMarkerField, StaticSchemaMarkerGraph,
84+
DynamicSchemaMarker, DynamicSchemaMarkerField, DynamicSchemaMarkerFieldFormat,
85+
DynamicSchemaMarkerGraph, DynamicSchemaMarkerSchema, FlowId, GraphColor, Marker, MarkerField,
86+
MarkerFieldKind, MarkerFlowFieldFormat, MarkerGraph, MarkerGraphType, MarkerHandle,
87+
MarkerLocations, MarkerNumberFieldFormat, MarkerStringFieldFormat, MarkerTiming,
88+
MarkerTypeHandle, Schema,
8889
};
8990
pub use native_symbols::NativeSymbolHandle;
9091
pub use process::ThreadHandle;

fxprof-processed-profile/src/marker_table.rs

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
use serde::ser::{Serialize, SerializeMap, SerializeSeq, Serializer};
22

3-
use crate::markers::{InternalMarkerSchema, MarkerFieldFormatKind};
3+
use crate::markers::{InternalMarkerSchema, MarkerFieldValueConsumer};
44
use crate::serialization_helpers::SerializableOptionalTimestampColumn;
55
use crate::string_table::{ProfileStringTable, StringHandle};
66
use crate::{
7-
CategoryHandle, Marker, MarkerFieldFormat, MarkerHandle, MarkerTiming, MarkerTypeHandle,
8-
Timestamp,
7+
CategoryHandle, DynamicSchemaMarker, DynamicSchemaMarkerFieldFormat, MarkerHandle,
8+
MarkerStringFieldFormat, MarkerTiming, MarkerTypeHandle, Timestamp,
99
};
1010

1111
#[derive(Debug, Clone, Default)]
@@ -53,7 +53,7 @@ impl MarkerTable {
5353
}
5454

5555
#[allow(clippy::too_many_arguments)]
56-
pub fn add_marker<T: Marker>(
56+
pub fn add_marker<T: DynamicSchemaMarker>(
5757
&mut self,
5858
string_table: &mut ProfileStringTable,
5959
name_string_index: StringHandle,
@@ -75,25 +75,20 @@ impl MarkerTable {
7575
self.marker_phases.push(phase);
7676
self.marker_type_handles.push(marker_type_handle);
7777
self.marker_stacks.push(None);
78-
for (field_index, field) in schema.fields().iter().enumerate() {
79-
match field.format.kind() {
80-
MarkerFieldFormatKind::String => {
81-
let string_index = marker.string_field_value(field_index as u32);
82-
self.marker_field_string_values.push(string_index);
83-
}
84-
MarkerFieldFormatKind::Number => {
85-
let number_value = marker.number_field_value(field_index as u32);
86-
self.marker_field_number_values.push(number_value)
87-
}
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-
}
95-
}
96-
}
78+
79+
let MarkerTable {
80+
marker_field_string_values,
81+
marker_field_number_values,
82+
marker_field_flow_values,
83+
..
84+
} = self;
85+
86+
marker.push_field_values(&mut MarkerTableFieldValueConsumer {
87+
marker_field_string_values,
88+
marker_field_number_values,
89+
marker_field_flow_values,
90+
string_table,
91+
});
9792

9893
MarkerHandle(self.marker_categories.len() - 1)
9994
}
@@ -124,6 +119,30 @@ impl MarkerTable {
124119
}
125120
}
126121

122+
struct MarkerTableFieldValueConsumer<'a> {
123+
marker_field_string_values: &'a mut Vec<StringHandle>,
124+
marker_field_number_values: &'a mut Vec<f64>,
125+
marker_field_flow_values: &'a mut Vec<StringHandle>,
126+
string_table: &'a mut ProfileStringTable,
127+
}
128+
129+
impl<'a> MarkerFieldValueConsumer for MarkerTableFieldValueConsumer<'a> {
130+
fn consume_string_field(&mut self, string_handle: StringHandle) {
131+
self.marker_field_string_values.push(string_handle);
132+
}
133+
134+
fn consume_number_field(&mut self, number: f64) {
135+
self.marker_field_number_values.push(number);
136+
}
137+
138+
fn consume_flow_field(&mut self, flow: u64) {
139+
// Convert flow ID to hex string and store as StringHandle
140+
let hex_string = format!("{flow:x}");
141+
let flow_string_handle = self.string_table.index_for_string(&hex_string);
142+
self.marker_field_flow_values.push(flow_string_handle);
143+
}
144+
}
145+
127146
struct SerializableMarkerTable<'a> {
128147
marker_table: &'a MarkerTable,
129148
string_table: &'a ProfileStringTable,
@@ -215,23 +234,23 @@ impl Serialize for SerializableMarkerDataElement<'_> {
215234
map.serialize_entry("cause", &SerializableMarkerCause(*stack_index))?;
216235
}
217236
for field in schema.fields() {
218-
match field.format.kind() {
219-
MarkerFieldFormatKind::String => {
237+
match &field.format {
238+
DynamicSchemaMarkerFieldFormat::String(format) => {
220239
let value;
221240
(value, string_fields) = string_fields.split_first().unwrap();
222-
if field.format == MarkerFieldFormat::String {
241+
if *format == MarkerStringFieldFormat::String {
223242
map.serialize_entry(&field.key, value)?;
224243
} else {
225244
let str_val = string_table.get_string(*value);
226245
map.serialize_entry(&field.key, str_val)?;
227246
}
228247
}
229-
MarkerFieldFormatKind::Number => {
248+
DynamicSchemaMarkerFieldFormat::Number(_) => {
230249
let value;
231250
(value, number_fields) = number_fields.split_first().unwrap();
232251
map.serialize_entry(&field.key, value)?;
233252
}
234-
MarkerFieldFormatKind::Flow => {
253+
DynamicSchemaMarkerFieldFormat::Flow(_) => {
235254
let value;
236255
(value, flow_fields) = flow_fields.split_first().unwrap();
237256
map.serialize_entry(&field.key, value)?;

0 commit comments

Comments
 (0)