Skip to content

Commit e4f5d67

Browse files
mstangemstange
authored
Merge pull request #508 from mstange/fix-other-subcategory
Fix some issues related to categories and subcategories in fxprof-processed-profile
2 parents 668db54 + e4d4306 commit e4f5d67

File tree

6 files changed

+59
-108
lines changed

6 files changed

+59
-108
lines changed

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

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

33
use super::category_color::CategoryColor;
44

@@ -23,76 +23,64 @@ impl Serialize for CategoryHandle {
2323
///
2424
/// Subategories can be created with [`Profile::add_subcategory`](crate::Profile::add_subcategory).
2525
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)]
26-
pub struct SubcategoryIndex(pub u8);
26+
pub struct SubcategoryIndex(pub u16);
27+
28+
impl SubcategoryIndex {
29+
/// The "Other" subcategory. All categories have this subcategory as their first subcategory.
30+
pub const OTHER: Self = SubcategoryIndex(0);
31+
}
2732

2833
/// A profiling category pair, consisting of a category and an optional subcategory. Can be set on stack frames and markers.
2934
///
3035
/// Category pairs can be created with [`Profile::add_subcategory`](crate::Profile::add_subcategory)
3136
/// and from a [`CategoryHandle`].
3237
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)]
33-
pub struct CategoryPairHandle(
34-
pub(crate) CategoryHandle,
35-
pub(crate) Option<SubcategoryIndex>,
36-
);
38+
pub struct CategoryPairHandle(pub(crate) CategoryHandle, pub(crate) SubcategoryIndex);
3739

3840
impl From<CategoryHandle> for CategoryPairHandle {
3941
fn from(category: CategoryHandle) -> Self {
40-
CategoryPairHandle(category, None)
42+
CategoryPairHandle(category, SubcategoryIndex::OTHER)
4143
}
4244
}
4345

4446
/// The information about a category.
4547
#[derive(Debug)]
46-
pub struct Category {
47-
pub name: String,
48-
pub color: CategoryColor,
49-
pub subcategories: Vec<String>,
48+
pub struct InternalCategory {
49+
name: String,
50+
color: CategoryColor,
51+
subcategories: Vec<String>,
5052
}
5153

52-
impl Category {
54+
impl InternalCategory {
55+
pub fn new(name: String, color: CategoryColor) -> Self {
56+
let subcategories = vec!["Other".to_string()];
57+
Self {
58+
name,
59+
color,
60+
subcategories,
61+
}
62+
}
63+
5364
/// Add a subcategory to this category.
5465
pub fn add_subcategory(&mut self, subcategory_name: String) -> SubcategoryIndex {
55-
let subcategory_index = SubcategoryIndex(u8::try_from(self.subcategories.len()).unwrap());
66+
let subcategory_index = SubcategoryIndex(u16::try_from(self.subcategories.len()).unwrap());
5667
self.subcategories.push(subcategory_name);
5768
subcategory_index
5869
}
5970
}
6071

61-
impl Serialize for Category {
72+
impl Serialize for InternalCategory {
6273
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
63-
let mut subcategories = self.subcategories.clone();
64-
subcategories.push("Other".to_string());
65-
6674
let mut map = serializer.serialize_map(None)?;
6775
map.serialize_entry("name", &self.name)?;
6876
map.serialize_entry("color", &self.color)?;
69-
map.serialize_entry("subcategories", &subcategories)?;
77+
map.serialize_entry("subcategories", &self.subcategories)?;
7078
map.end()
7179
}
7280
}
7381

74-
#[derive(Debug, Clone)]
75-
pub enum Subcategory {
76-
Normal(SubcategoryIndex),
77-
Other(CategoryHandle),
78-
}
79-
80-
pub struct SerializableSubcategoryColumn<'a>(pub &'a [Subcategory], pub &'a [Category]);
81-
82-
impl Serialize for SerializableSubcategoryColumn<'_> {
82+
impl Serialize for SubcategoryIndex {
8383
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
84-
let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
85-
for subcategory in self.0 {
86-
match subcategory {
87-
Subcategory::Normal(index) => seq.serialize_element(&index.0)?,
88-
Subcategory::Other(category) => {
89-
// There is an implicit "Other" subcategory at the end of each category's
90-
// subcategory list.
91-
let subcategory_count = self.1[category.0 as usize].subcategories.len();
92-
seq.serialize_element(&subcategory_count)?
93-
}
94-
}
95-
}
96-
seq.end()
84+
self.0.serialize(serializer)
9785
}
9886
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use serde::ser::{Serialize, Serializer};
22

33
/// One of the available colors for a category.
4-
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq)]
4+
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)]
55
pub enum CategoryColor {
66
Transparent,
77
LightBlue,

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

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

3-
use crate::category::{
4-
Category, CategoryHandle, CategoryPairHandle, SerializableSubcategoryColumn, Subcategory,
5-
};
3+
use crate::category::{CategoryHandle, CategoryPairHandle, SubcategoryIndex};
64
use crate::fast_hash_map::FastHashMap;
75
use crate::frame::FrameFlags;
86
use crate::func_table::{FuncIndex, FuncTable};
@@ -16,7 +14,7 @@ use crate::thread_string_table::{ThreadInternalStringIndex, ThreadStringTable};
1614
pub struct FrameTable {
1715
addresses: Vec<Option<u32>>,
1816
categories: Vec<CategoryHandle>,
19-
subcategories: Vec<Subcategory>,
17+
subcategories: Vec<SubcategoryIndex>,
2018
funcs: Vec<FuncIndex>,
2119
native_symbols: Vec<Option<NativeSymbolIndex>>,
2220
internal_frame_to_frame_index: FastHashMap<InternalFrame, usize>,
@@ -86,11 +84,7 @@ impl FrameTable {
8684
};
8785
let func_index =
8886
func_table.index_for_func(location_string_index, resource, frame.flags);
89-
let CategoryPairHandle(category, subcategory_index) = frame.category_pair;
90-
let subcategory = match subcategory_index {
91-
Some(index) => Subcategory::Normal(index),
92-
None => Subcategory::Other(category),
93-
};
87+
let CategoryPairHandle(category, subcategory) = frame.category_pair;
9488
addresses.push(address);
9589
categories.push(category);
9690
subcategories.push(subcategory);
@@ -99,37 +93,22 @@ impl FrameTable {
9993
frame_index
10094
})
10195
}
102-
103-
pub fn as_serializable<'a>(&'a self, categories: &'a [Category]) -> impl Serialize + 'a {
104-
SerializableFrameTable {
105-
table: self,
106-
categories,
107-
}
108-
}
10996
}
11097

111-
struct SerializableFrameTable<'a> {
112-
table: &'a FrameTable,
113-
categories: &'a [Category],
114-
}
115-
116-
impl Serialize for SerializableFrameTable<'_> {
98+
impl Serialize for FrameTable {
11799
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
118-
let len = self.table.addresses.len();
100+
let len = self.addresses.len();
119101
let mut map = serializer.serialize_map(None)?;
120102
map.serialize_entry("length", &len)?;
121103
map.serialize_entry(
122104
"address",
123-
&SerializableFrameTableAddressColumn(&self.table.addresses),
105+
&SerializableFrameTableAddressColumn(&self.addresses),
124106
)?;
125107
map.serialize_entry("inlineDepth", &SerializableSingleValueColumn(0u32, len))?;
126-
map.serialize_entry("category", &self.table.categories)?;
127-
map.serialize_entry(
128-
"subcategory",
129-
&SerializableSubcategoryColumn(&self.table.subcategories, self.categories),
130-
)?;
131-
map.serialize_entry("func", &self.table.funcs)?;
132-
map.serialize_entry("nativeSymbol", &self.table.native_symbols)?;
108+
map.serialize_entry("category", &self.categories)?;
109+
map.serialize_entry("subcategory", &self.subcategories)?;
110+
map.serialize_entry("func", &self.funcs)?;
111+
map.serialize_entry("nativeSymbol", &self.native_symbols)?;
133112
map.serialize_entry("innerWindowID", &SerializableSingleValueColumn((), len))?;
134113
map.serialize_entry("implementation", &SerializableSingleValueColumn((), len))?;
135114
map.serialize_entry("line", &SerializableSingleValueColumn((), len))?;

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

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::time::Duration;
55
use serde::ser::{Serialize, SerializeMap, SerializeSeq, Serializer};
66
use serde_json::json;
77

8-
use crate::category::{Category, CategoryHandle, CategoryPairHandle};
8+
use crate::category::{CategoryHandle, CategoryPairHandle, InternalCategory};
99
use crate::category_color::CategoryColor;
1010
use crate::counters::{Counter, CounterHandle};
1111
use crate::cpu_delta::CpuDelta;
@@ -120,8 +120,8 @@ pub struct Profile {
120120
pub(crate) interval: SamplingInterval,
121121
pub(crate) global_libs: GlobalLibTable,
122122
pub(crate) kernel_libs: LibMappings<LibraryHandle>,
123-
pub(crate) categories: Vec<Category>, // append-only for stable CategoryHandles
124-
pub(crate) processes: Vec<Process>, // append-only for stable ProcessHandles
123+
pub(crate) categories: Vec<InternalCategory>, // append-only for stable CategoryHandles
124+
pub(crate) processes: Vec<Process>, // append-only for stable ProcessHandles
125125
pub(crate) counters: Vec<Counter>,
126126
pub(crate) threads: Vec<Thread>, // append-only for stable ThreadHandles
127127
pub(crate) initial_visible_threads: Vec<ThreadHandle>,
@@ -160,11 +160,10 @@ impl Profile {
160160
processes: Vec::new(),
161161
string_table: GlobalStringTable::new(),
162162
marker_schemas: Vec::new(),
163-
categories: vec![Category {
164-
name: "Other".to_string(),
165-
color: CategoryColor::Gray,
166-
subcategories: Vec::new(),
167-
}],
163+
categories: vec![InternalCategory::new(
164+
"Other".to_string(),
165+
CategoryColor::Gray,
166+
)],
168167
static_schema_marker_types: FastHashMap::default(),
169168
symbolicated: false,
170169
used_pids: FastHashMap::default(),
@@ -198,11 +197,8 @@ impl Profile {
198197
/// Categories are used for stack frames and markers, as part of a "category pair".
199198
pub fn add_category(&mut self, name: &str, color: CategoryColor) -> CategoryHandle {
200199
let handle = CategoryHandle(self.categories.len() as u16);
201-
self.categories.push(Category {
202-
name: name.to_string(),
203-
color,
204-
subcategories: Vec::new(),
205-
});
200+
self.categories
201+
.push(InternalCategory::new(name.to_string(), color));
206202
handle
207203
}
208204

@@ -212,7 +208,7 @@ impl Profile {
212208
/// its corresponding `CategoryPairHandle` for the default category using `category.into()`.
213209
pub fn add_subcategory(&mut self, category: CategoryHandle, name: &str) -> CategoryPairHandle {
214210
let subcategory = self.categories[category.0 as usize].add_subcategory(name.into());
215-
CategoryPairHandle(category, Some(subcategory))
211+
CategoryPairHandle(category, subcategory)
216212
}
217213

218214
/// Add an empty process. The name, pid and start time can be changed afterwards,
@@ -986,7 +982,6 @@ impl Profile {
986982
SerializableProfileThreadsProperty {
987983
threads: &self.threads,
988984
processes: &self.processes,
989-
categories: &self.categories,
990985
sorted_threads,
991986
marker_schemas: &self.marker_schemas,
992987
global_string_table: &self.string_table,
@@ -1104,7 +1099,6 @@ impl Serialize for SerializableProfileMeta<'_> {
11041099
struct SerializableProfileThreadsProperty<'a> {
11051100
threads: &'a [Thread],
11061101
processes: &'a [Process],
1107-
categories: &'a [Category],
11081102
sorted_threads: &'a [ThreadHandle],
11091103
marker_schemas: &'a [InternalMarkerSchema],
11101104
global_string_table: &'a GlobalStringTable,
@@ -1115,15 +1109,13 @@ impl Serialize for SerializableProfileThreadsProperty<'_> {
11151109
let mut seq = serializer.serialize_seq(Some(self.threads.len()))?;
11161110

11171111
for thread in self.sorted_threads {
1118-
let categories = self.categories;
11191112
let thread = &self.threads[thread.0];
11201113
let process = &self.processes[thread.process().0];
11211114
let marker_schemas = self.marker_schemas;
11221115
let global_string_table = self.global_string_table;
11231116
seq.serialize_element(&SerializableProfileThread(
11241117
process,
11251118
thread,
1126-
categories,
11271119
marker_schemas,
11281120
global_string_table,
11291121
))?;
@@ -1154,27 +1146,19 @@ impl Serialize for SerializableProfileCountersProperty<'_> {
11541146
struct SerializableProfileThread<'a>(
11551147
&'a Process,
11561148
&'a Thread,
1157-
&'a [Category],
11581149
&'a [InternalMarkerSchema],
11591150
&'a GlobalStringTable,
11601151
);
11611152

11621153
impl Serialize for SerializableProfileThread<'_> {
11631154
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
1164-
let SerializableProfileThread(
1165-
process,
1166-
thread,
1167-
categories,
1168-
marker_schemas,
1169-
global_string_table,
1170-
) = self;
1155+
let SerializableProfileThread(process, thread, marker_schemas, global_string_table) = self;
11711156
let process_start_time = process.start_time();
11721157
let process_end_time = process.end_time();
11731158
let process_name = process.name();
11741159
let pid = process.pid();
11751160
thread.serialize_with(
11761161
serializer,
1177-
categories,
11781162
process_start_time,
11791163
process_end_time,
11801164
process_name,

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::cmp::Ordering;
33

44
use serde::ser::{SerializeMap, Serializer};
55

6-
use crate::category::Category;
76
use crate::cpu_delta::CpuDelta;
87
use crate::frame_table::{FrameTable, InternalFrame};
98
use crate::func_table::FuncTable;
@@ -214,7 +213,6 @@ impl Thread {
214213
pub fn serialize_with<S: Serializer>(
215214
&self,
216215
serializer: S,
217-
categories: &[Category],
218216
process_start_time: Timestamp,
219217
process_end_time: Option<Timestamp>,
220218
process_name: &str,
@@ -232,7 +230,7 @@ impl Thread {
232230
let thread_unregister_time = self.end_time;
233231

234232
let mut map = serializer.serialize_map(None)?;
235-
map.serialize_entry("frameTable", &self.frame_table.as_serializable(categories))?;
233+
map.serialize_entry("frameTable", &self.frame_table)?;
236234
map.serialize_entry("funcTable", &self.func_table)?;
237235
map.serialize_entry(
238236
"markers",

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,7 +1021,8 @@ fn profile_with_js() {
10211021
);
10221022

10231023
let some_label_string = profile.intern_string("Some label string");
1024-
let category = profile.add_category("Regular", CategoryColor::Green);
1024+
let category = profile.add_category("Cycle Collection", CategoryColor::Orange);
1025+
let category_pair = profile.add_subcategory(category, "Graph Reduction");
10251026
let s1 = profile.intern_stack_frames(
10261027
thread,
10271028
vec![
@@ -1032,7 +1033,7 @@ fn profile_with_js() {
10321033
},
10331034
FrameInfo {
10341035
frame: Frame::ReturnAddress(0x7f76b7ffc0e7),
1035-
category_pair: category.into(),
1036+
category_pair,
10361037
flags: FrameFlags::empty(),
10371038
},
10381039
]
@@ -1061,10 +1062,11 @@ fn profile_with_js() {
10611062
]
10621063
},
10631064
{
1064-
"name": "Regular",
1065-
"color": "green",
1065+
"name": "Cycle Collection",
1066+
"color": "orange",
10661067
"subcategories": [
1067-
"Other"
1068+
"Other",
1069+
"Graph Reduction"
10681070
]
10691071
}
10701072
],
@@ -1112,7 +1114,7 @@ fn profile_with_js() {
11121114
],
11131115
"subcategory": [
11141116
0,
1115-
0
1117+
1
11161118
],
11171119
"func": [
11181120
0,

0 commit comments

Comments
 (0)