Closed Bug 1077449 Opened 5 years ago Closed 5 years ago

Display Framerate data in new performance tool

Categories

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

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 36

People

(Reporter: jsantell, Assigned: jsantell)

References

Details

Attachments

(2 files, 1 obsolete file)

No description provided.
Assignee: nobody → jsantell
Depends on: 1077447, perf-tool-v2
Blocks: 1077447
No longer depends on: 1077447
Blocks: perf-tool-v2
No longer blocks: 1077447
Depends on: 1077447
No longer depends on: perf-tool-v2
The `Overview` view requires the use of a `timeline` actor, introduced in 35. Within that, it uses the memory actor (way older gecko?) and framerate actor (Gecko 33). The API changed for framerate actor before 35, so we will only display the overview view at all when the timeline actor exists.
Attached patch 1077449-realtime-fps.patch (obsolete) — Splinter Review
Finally sometime to work on this again. Rough idea, but seems to be working. More magic will have to be done to sync this up to the other time domain graphs in the overview later.

Attachment #8524123 - Flags: review?(vporof)
Just built this, the tool looks really weird when you first start it. Should probably do something to hide the tooltips or the whole graph while not recording.
Comment on attachment 8524123 [details] [diff] [review]
1077449-realtime-fps.patch

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

Couple of general concerns:

1. The selection should be disabled while recording.

2. The selection should be cleared when a new recording is started.

3. The whole browser becomes unresponsive after 30 or so seconds of recording, because looping over the giant array of timestamps takes forever. I doubt that painting the canvas itself is slow, but simply iterating takes too long and freezes the browser. Therefore,
 -- Building the data source should be moved in a worker, most likely.
 -- The data source should be filtered sparsely enough so that looping over it in the main thread doesn't take long; frame rate measurements that are too close to each other don't need to be drawn at 1x zoom. Currently, the graph handles this (via `minDistanceBetweenPoints`), but it's on the main thread, therefore sadness.
 -- I also think that we may be able to do off-the-main thread canvas operations now. Bug 709490 comes to mind, but it might be true for canvas 2D as well.

4. This needs to survive reloads. There's no reason it can't, because the framerate actor does requestAnimationFrame on the chrome window, not the content.

Code-wise I have nothing to complain about.

::: browser/devtools/performance/test/browser_perf-overview-render-01.js
@@ +10,5 @@
> +
> +  yield startRecording(panel);
> + 
> +  yield once(OverviewView, EVENTS.OVERVIEW_RENDERED);
> +  yield once(OverviewView, EVENTS.OVERVIEW_RENDERED);

Why twice?
Attachment #8524123 - Flags: review?(vporof) → review-
Added bug 1101146 for having an initial panel covering everything in the performance tool until an action occurs.
Comment on attachment 8524123 [details] [diff] [review]
1077449-realtime-fps.patch

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

::: browser/devtools/performance/test/browser_perf-overview-render-01.js
@@ +10,5 @@
> +
> +  yield startRecording(panel);
> + 
> +  yield once(OverviewView, EVENTS.OVERVIEW_RENDERED);
> +  yield once(OverviewView, EVENTS.OVERVIEW_RENDERED);

Just to ensure the timeout loop is working
r+'d via victor in irc

1. Selection now disabled during recording
2. Selection now cleared when restarting recording
3. Performance issues with large data being iterated over for the graph will be handled in bug 1101197
4. Issue with surviving reload, created bug 1101191, issue in framerate actor itself

Also added tests for the selection scenarios above.

trypatch: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=877b3bee68a9
Attachment #8524123 - Attachment is obsolete: true
Attachment #8524901 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/d6dbc6e05dce
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.