Show size units in the timeline for size profiles#5364
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5364 +/- ##
==========================================
+ Coverage 85.84% 85.90% +0.06%
==========================================
Files 312 312
Lines 29827 29893 +66
Branches 8213 8245 +32
==========================================
+ Hits 25605 25680 +75
+ Misses 3627 3621 -6
+ Partials 595 592 -3 ☔ View full report in Codecov by Sentry. |
|
I think this is cool idea. But I think it shouldn't be magically based on |
7a51f99 to
63ccd74
Compare
mstange
left a comment
There was a problem hiding this comment.
Functionality looks good, all my change requests are about types, function names and tests.
| ['bytes', 123456789], | ||
| ['bytes', 123456789123], |
There was a problem hiding this comment.
(no action needed:) It's interesting to me that format-numbers.js is tested via the marker schema. It would make more sense to me to have these tests in format-numbers.test.js but I guess you're just following what's already there.
63ccd74 to
2caf554
Compare
mstange
left a comment
There was a problem hiding this comment.
Looks good to me!
I should acknowledge that it's going to be confusing to have many variables with time or Milliseconds in them which can now store values in bytes. Can you file a follow-up issue to find a way to clean this up (for example by renaming time to offset)?
2caf554 to
bb13a14
Compare
… weightType is bytes.
bb13a14 to
c92382d
Compare
…tools#5364 in the upgrader and changelog for version 54.
…tools#5364 in the upgrader and changelog for version 54.
…hangelog for version 54. (#5384)
[Julien Wajsberg] Update node to v22 (#5378) [Markus Stange] Speed up createCallNodeTable by 2.3x (#5248) [Markus Stange] Remove frameTable.implementation from the processed format. (#5370) [Florian Quèze] Show size units in the timeline for profiles where profile.meta.sampleUnits.time is "bytes". (#5364) [Sean Kim] Report nsIRequest::status (nsresult) in the marker (#5375) [Markus Stange] Change the marker schema to accept a description and get rid of the notion of static fields (#5385) Also thanks to our localizers: en-CA: Paul tr: Grk
|
looks like this was forgotten: profiler/src/components/stack-chart/Canvas.js Line 487 in 90bb6f3 |
Example profile: https://share.firefox.dev/42Kf0Ju