Closed Bug 1144423 Opened 5 years ago Closed 5 years ago

Allocations tree should have total %/bytes and self %/bytes in addition to counts

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect)

x86
macOS
defect
Not set

Tracking

(firefox43 fixed)

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: fitzgen, Assigned: jsantell)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We shouldn't make people do the math in their heads...
Not specifically blocking v2.
No longer blocks: perf-tool-v2
Here is my ideal column setup, where we do merging of the raw number of a measurement and the percent of all measurements, similar to how chrome does:

+-------------+------------+-------------+-------------+------------------------------+
| Self Bytes  | Self Count | Total Bytes | Total Count | Function                     |
+-------------+------------+-------------+-------------+------------------------------+
| 1790272 41% | 8307   17% | 1790372 42% | 8317    18% | V someFunc @ a.j:345:6       |
|     100  1% | 10      1% |     100  1% |   10     1% |   > callerFunc @ b.j:765:34  |
+-------------+------------+-------------+-------------+------------------------------+
What're your thoughts on pretty printing byte size, like "10.6 MB"
-0

Biggest desire is to be able to easily compare numbers without needing to check the unit, and for the numbers to line up properly when stacked on top of each other.

Eg, I want this (which is easy to tell which is bigger at a glance:

> 10000
>  1000
>   100
>    10
>     1

Rather than this, where the numbers no longer match up:

> 10MB
>  1KB
> 900B

Maybe my worries are unfounded, though?
Can see both use cases for high and low level views. Let's start with bytes and can change it if we need, in lieu of time
(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #5)
> Let's start with bytes and can change it if we need, in lieu of time

+1
Summary: Allocations tree should have total % and self % in addition to counts → Allocations tree should have total %/bytes and self %/bytes in addition to counts
Assignee: nobody → jsantell
Depends on: 1191508
Depends on: 1192335
Blocks: perf-tools-fx43
No longer blocks: perf-tools-fx42
Depends on: 1194458
Victor for code, Fitzgen for correctness
https://treeherder.mozilla.org/#/jobs?repo=try&revision=414b31fb962a
Attachment #8652956 - Flags: review?(vporof)
Attachment #8652956 - Flags: review?(nfitzgerald)
Comment on attachment 8652956 [details] [diff] [review]
1144423-display-bytes.patch

Review of attachment 8652956 [details] [diff] [review]:
-----------------------------------------------------------------

r=me in that the data looks like it is presented right

However, we can't ship with the performance situation like it is now. I'm seeing 10 second beachballs when going to the allocations tab, every time I Cmd-TAB away from firefox and back again, I get another 3 seconds or so of beachballing, and expanding tree items gives another second or so of jank every time. And this is on a top of the line developer machine.

STR:

* Load cnn.com
* Start recording w/ allocations recording
* Refresh cnn.com
* Wait till it is loaded all the way
* Stop recording
* Switch to allocations tab
Attachment #8652956 - Flags: review?(nfitzgerald) → review+
there's a few bugs for improving the tree views performance (bug 1164137, bug 1169035, biggest one being rendering only visible elements bug 1185181), I think Victor's checking out bug 1185181.

Yeah it's a ton of data. The performance sucks.
Up to ya'll if we should land this -- I think it's better than what we have right now, but wouldn't mind holding off on enabling the option in aurora, blogging/marketing this for another cycle for perf improvements/polish, etc.
Attachment #8652956 - Flags: review?(vporof) → review+
https://hg.mozilla.org/mozilla-central/rev/944ac81b4b76
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
In terms of performance, the following list is not especially optimal:
.call-tree-header[type="allocations"],
.call-tree-cell[type="allocations"],
.call-tree-header[type="self-allocations"],
.call-tree-cell[type="self-allocations"],
.call-tree-header[type="count"],
.call-tree-cell[type="count"],
.call-tree-header[type="self-count"],
.call-tree-cell[type="self-count"],
.call-tree-header[type="size"],
.call-tree-cell[type="size"],
.call-tree-header[type="self-size"],
.call-tree-cell[type="self-size"],
.call-tree-header[type="count-percentage"],
.call-tree-cell[type="count-percentage"],
.call-tree-header[type="self-count-percentage"],
.call-tree-cell[type="self-count-percentage"],
.call-tree-header[type="size-percentage"],
.call-tree-cell[type="size-percentage"],
.call-tree-header[type="self-size-percentage"],
.call-tree-cell[type="self-size-percentage"]{
	width:6vw}

Possibly better to have class "number" or so?
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.