FPS drops to 0 when opening an app

RESOLVED FIXED in Firefox 40

Status

defect
P1
normal
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: paul, Assigned: jsantell)

Tracking

Trunk
Firefox 41
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed, firefox41 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Posted image screenshot
When profiling the main process, the FPS graph drops to 0. Stays at 0 even if I close the app.
Apparently this isn't only affecting FxOS
Summary: FxOS / Perf++: FPS drops to 0 when opening an app → FPS drops to 0 when opening an app
Priority: -- → P1
I've had this happen before as well, but not sure how to consistently recreate it. Any ideas?
Posted file 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.
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.
Blocks: 1158645
No longer blocks: 1158645
Need to wait for bug 1152829 and a unified way of measuring time, so we don't rely on performance.now() anymore.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
This is important as this will be in 40.0.
Depends on: 1152829
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.
No longer depends on: 1152829
See Also: → 1152829
Posted patch 1146237-framerate-actor.patch (obsolete) — Splinter Review
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.
Assignee: vporof → jsantell
Attachment #8610862 - Flags: review?(vporof)
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?
Attachment #8610862 - Flags: review?(vporof) → review+
(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
Just use tabActor.docShell.
Attachment #8610862 - Attachment is obsolete: true
Attachment #8611449 - Flags: review+
Duplicate of this bug: 1165151
https://hg.mozilla.org/mozilla-central/rev/4e7838090e03
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
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
Attachment #8611449 - Flags: approval-mozilla-aurora?
Attachment #8611449 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.