Show meta category nodes instead of filtering them out in the flame graph

RESOLVED FIXED

Status

()

Firefox
Developer Tools: Performance Tools (Profiler/Timeline)
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jsantell, Assigned: vporof)

Tracking

(Blocks: 1 bug)

37 Branch
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Like bug 1108843, but for the flame graph.
(Reporter)

Updated

3 years ago
Assignee: nobody → jsantell
Blocks: 1145697
Status: NEW → ASSIGNED
Depends on: 1108843
(Assignee)

Updated

3 years ago
No longer depends on: 1108843
(Reporter)

Updated

3 years ago
Depends on: 1108843
(Reporter)

Updated

3 years ago
Depends on: 1138928
Created attachment 8590518 [details] [diff] [review]
1147693-flamegraphmeta.patch

Needs tests. flame graph utils tests are the worst.
(Assignee)

Updated

3 years ago
Assignee: jsantell → vporof
(Reporter)

Updated

3 years ago
Priority: -- → P2

Comment 2

3 years ago
I think I fixed this with bug 1154115.
This looks like it's good to go -- but not sure if we have tests for this?

Comment 4

3 years ago
(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #3)
> This looks like it's good to go -- but not sure if we have tests for this?

Kinda. Some of the old flame graph tests were asserting on the exact output of that buildFlameGraphDataFromSamples function or whatever it's called. Since that function now outputs metacategories, I had to change those tests to expect those new nodes.
SGTM.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Blocks: 1167252

Updated

2 years ago
Flags: qe-verify+
This doesn't need an uplift, right?  Looks like it's been resolved without a changeset
No longer blocks: 1167252
Flags: needinfo?(jsantell)
Correct, was fixed as a part of shu's rewrite
Flags: needinfo?(jsantell)
You need to log in before you can comment on or make changes to this bug.