Closed Bug 1150761 Opened 9 years ago Closed 9 years ago

Rename Details Views

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(firefox40 verified, firefox41 fixed)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- fixed

People

(Reporter: fitzgen, Assigned: jsantell)

References

Details

Attachments

(1 file, 1 obsolete file)

Once bug 1108843 lands we won't only be showing JS here and the biggest differentiator it has from the flame graph is that it is a tree/table rather than a visualization. They both show pretty much the same data so it isn't fair to say one is "JavaScript" but the other isn't. Either way, it isn't super descriptive.

"Call Tree" ?

"Functions Tree" ?

"Functions Table" ?

Simply "Table" or "Tree" ?
Whatever we call it, we would also probably need to update the "JS Flame Chart" as well -- we name the other two views "Allocations" and "Allocations Flame Chart" -- what do the "JavaScript" and "JS Flame Chart" have in common to call that? Chrome categorizes these as "JavaScript CPU Profiles" but I think we can name it something more accurate.
The JS ones just have time as their x-axis / time as the thing being profiled.

The allocations ones (should) have allocations as the thing being profiled, but I think it doesn't do the right thing right now.
If we're doing this, we need to rename this now for 40.1 and get strings in before Fx40.
Moving "Timeline" renaming bug 1166499 here. Read that for previous discussions.

My vote is for Waterfall, Call Tree and Flame Chart. These describe the presentation of the data, as they are all the same source (marker vs sampler data is a black box, and we do not reveal this to the end user). "Timeline" conflicts with the *main timeline that controls all views and used for scrubbing and selecting*, and "JavaScript" doesn't tell me much more about the data -- it's obviously JS (or pseudoframes but whatever).

When we introduce allocations views, those can be "Allocation Tree" and "Allocation Flame" (because ideally we'll have flame GRAPHS as well, and can toggle between them).
Blocks: perf-tool-v2
Summary: Rename "JavaScript" tab to "Call Tree" or something → Rename Details Views
Whiteboard: [devedition-40]
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
I'll throw my vote in for Waterfall, call tree and flame chart. It's very intuitive.
Barring any changes, we should try to land this before week's end for the screencasts
Attachment #8609202 - Flags: review?(vporof)
Comment on attachment 8609202 [details] [diff] [review]
1150761-rename-details-view.patch

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

How about renaming the memory columns too while we're here?
Attachment #8609202 - Flags: review?(vporof) → review+
Allocations Call Tree and Allocations Flame Chart?
Allocations Tree and Allocations Chart.
(In reply to Victor Porof [:vporof][:vp] from comment #10)
> Allocations Tree and Allocations Chart.

Yeah, +1
Added allocation naming
Attachment #8609202 - Attachment is obsolete: true
Attachment #8609515 - Flags: review+
(In reply to Victor Porof [:vporof][:vp] from comment #10)
> Allocations Tree and Allocations Chart.

+2
Whiteboard: [devedition-40] → [devedition-40][fixed-in-fx-team]
Is the plan to land another patch on m-c (but not to be uplifted) that properly changes these strings?  May be easiest to open another bug for that and not mark it as blocking the uplift bug.
Flags: needinfo?(jsantell)
Added another entry in the l10n meta bug noting these changes -- don't want to deal with more merging issues for a potential 3 weeks headstart on these strings, more heartache for minimal gain
Flags: needinfo?(jsantell)
Yes, land on mc but post June 2nd to reduce heartache for minimal gain. Noted already in the meta l10n bug
https://hg.mozilla.org/mozilla-central/rev/1c539a7eb9b2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [devedition-40][fixed-in-fx-team] → [devedition-40]
Target Milestone: --- → Firefox 41
Flags: qe-verify+
Comment on attachment 8609515 [details] [diff] [review]
1150761-rename-details-view.patch


Approval Request Comment
[Feature/regressing bug #]: 1167252, the new performance tool
[User impact if declined]: Won't ship the performance tool
[Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252 with all patches needing uplift
[Risks and why]: Requesting uplift for the accumulated changes in the performance tool since the 40 merge date, so these changes haven't had the full 6 weeks to bake.  Risks are generally contained within devtools, specifically within the performance panel.
[String/UUID change made/needed]: None
Attachment #8609515 - Flags: approval-mozilla-aurora?
Note: I had verbal confirmation for these uplifts from Sylvestre even before he's flagged them as a+.  See https://bugzilla.mozilla.org/show_bug.cgi?id=1167252#c26
Comment on attachment 8609515 [details] [diff] [review]
1150761-rename-details-view.patch

Change approved to skip one train as part of the spring campaign.
Attachment #8609515 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on Aurora 40.0a2 (2015-06-08), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.

The detailed views from the Performance section are now called "Waterfall", "Call Tree" and "Flame Chart".
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [devedition-40] → [polish-backlog]
Whiteboard: [polish-backlog]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.