1146237 - FPS drops to 0 when opening an app

Status: RESOLVED FIXED

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

Description

[Paul Rouget [:paul]]

Attachment: screenshot

When profiling the main process, the FPS graph drops to 0. Stays at 0 even if I close the app.

Comment 1

[Victor Porof [:vporof][:vp]]

Apparently this isn't only affecting FxOS

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

I've had this happen before as well, but not sure how to consistently recreate it. Any ideas?

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: fps_flatline.json

got a recording of it. the frames data from the actor has about 680 values, and is a 15s recording. Inspecting the data there...

> json.ticks[200]
'3351.9978289999963'
> json.ticks[210]
'3519.1548000000003'
> json.ticks[211]
'3535.1599760000026'
> json.ticks[212]
'3552.061393'
> json.ticks[213]
'3568.217128999997'
> json.ticks[214]
'3596.334998999999'
> json.ticks[215]
'4347.913415999992'
> json.ticks[216]
'38827.078467'
> json.ticks[217]
'38827.13419699999'
> json.ticks[218]
'73939.084618'

So somewhere around there, it jumps from timestamps of 3s, to 4s, to 38s and then 73s, with the end of the array being at 83s.

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

I suspect this is why the average is skewed in bug 1158645, as all those values in the future are being counted, just not displayed.

Comment 5

[Victor Porof [:vporof][:vp]]

Need to wait for bug 1152829 and a unified way of measuring time, so we don't rely on performance.now() anymore.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

This is important as this will be in 40.0.

Comment 7

[Victor Porof [:vporof][:vp]]

Making bug 1152829 as see-also, since I'm not entirely sure this issue is caused because of our reliance on performance.now(). I'll investigate it anyway in the meantime.

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1146237-framerate-actor.patch

So this uses docShell.now(), which is what timeline actor uses.

The docShells getter is lifted right from the timeline actor. And that looks very similar to what the tab actor has in the first place. We should consolidate these somehow, AND if we only care about the main window for these, then cache the docshell we are interested in, because that can take up 1-2% in costs from what I can tell (just accessing TabActor's originalDocShell).

I don't know all the details with the docshell stuff though, so won't be able to get any nice refactoring in for this in time for 40.1, but hopefully this prevents flat lining.

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d5912c95aa8

Comment 10

[Victor Porof [:vporof][:vp]]

Comment on attachment 8610862 [details] [diff] [review]
1146237-framerate-actor.patch

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

::: toolkit/devtools/server/actors/framerate.js
@@ +127,5 @@
> +      let docShell = docShellsEnum.getNext();
> +      docShells.push(docShell.QueryInterface(Ci.nsIDocShell));
> +    }
> +
> +    return docShells;

Why return all the docshells instead of just the first one?

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

(In reply to Victor Porof [:vporof][:vp] from comment #10)
> Comment on attachment 8610862 [details] [diff] [review]
> 1146237-framerate-actor.patch
> 
> Review of attachment 8610862 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/devtools/server/actors/framerate.js
> @@ +127,5 @@
> > +      let docShell = docShellsEnum.getNext();
> > +      docShells.push(docShell.QueryInterface(Ci.nsIDocShell));
> > +    }
> > +
> > +    return docShells;
> 
> Why return all the docshells instead of just the first one?

Ah, timeline does that for markers on iframes, just no paint markers on those iframes, I misunderstood that. OK I can clean up this getter then in that case

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1146237-framerate-actor.patch

Just use tabActor.docShell.

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

https://hg.mozilla.org/integration/fx-team/rev/4e7838090e03

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4e7838090e03

Comment 16

[Brian Grinstead [:bgrins]]

Comment on attachment 8611449 [details] [diff] [review]
1146237-framerate-actor.patch

Approval Request Comment
[Feature/regressing bug #]: 1167252, the new performance tool
[User impact if declined]: The performance tool framerate will be wrong when reloading a page
[Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252
[Risks and why]: Minor - the risk is contained within devtools performance panel.
[String/UUID change made/needed]: None

Comment 17

[Sylvestre Ledru [:Sylvestre]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/86c124e3b2a9