Closed Bug 1074899 Opened 10 years ago Closed 10 years ago

Add self time and self cost to call tree

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 35

People

(Reporter: fitzgen, Assigned: fitzgen)

Details

Attachments

(1 file, 2 obsolete files)

Can't tell how much is in a given frame or its children without doing math. Math is hard.
Attached patch self-time.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=cb32702bb3d1
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Attachment #8498090 - Flags: review?(vporof)
Comment on attachment 8498090 [details] [diff] [review]
self-time.patch

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

Please rename "cost" to "total cost" and fix formatting for r+.

::: browser/devtools/profiler/utils/tree-view.js
@@ +69,5 @@
> +    let framePercentage = this._getPercentage(this.frame.samples);
> +    let selfPercentage = clamp(framePercentage - sum([this._getPercentage(c.samples)
> +                                                      for (c of this._getChildCalls())]),
> +                               0,
> +                               100);

What kind of formatting is this? r- forever. When did assigning stuff to variables become too mainstream?
Attachment #8498090 - Flags: review?(vporof) → review+
Attached patch self-time.patch (obsolete) — Splinter Review
Fixed nits.

Carrying over r+

https://tbpl.mozilla.org/?tree=Try&rev=9b9ad51afe3a
Attachment #8498090 - Attachment is obsolete: true
Attachment #8498110 - Flags: review+
Comment on attachment 8498110 [details] [diff] [review]
self-time.patch

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

::: browser/locales/en-US/chrome/browser/devtools/profiler.dtd
@@ +38,5 @@
>  <!ENTITY profilerUI.clearButton "Clear">
>  
>  <!-- LOCALIZATION NOTE (profilerUI.table.*): These strings are displayed
>    -  in the call tree headers for a recording. -->
> +<!ENTITY profilerUI.table.duration       "Total Time (ms)">

Rename profilerUI.table.duration to profilerUI.table.duration2

@@ +40,5 @@
>  <!-- LOCALIZATION NOTE (profilerUI.table.*): These strings are displayed
>    -  in the call tree headers for a recording. -->
> +<!ENTITY profilerUI.table.duration       "Total Time (ms)">
> +<!ENTITY profilerUI.table.selfDuration   "Self Time (ms)">
> +<!ENTITY profilerUI.table.percentage     "Total Cost">

Rename profilerUI.table.percentage to profilerUI.table.percentage2
Attachment #8498110 - Flags: review+
Attached patch self-time.patchSplinter Review
Opted for "totalDuration" instead of "duration2".
Attachment #8498196 - Flags: review?(vporof)
Attachment #8498196 - Flags: review?(vporof) → review+
Attachment #8498110 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/c3c65a614602
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c3c65a614602
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.