1503339 - Experiment with lowering frame rate on low-end devices

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[:Gijs (he/him)]

We're investigating lowering frame rate on low-end devices as a way to improve (perceived) performance.

Just setting the layout.frame_rate pref to 30, and running the raptor tp6 page load tests on the 2018 perf reference device, I'm seeing 10% improvements across the board on load times compared with the default (-1) setting.

Trying to write a proof-of-concept patch, I've run into 2 issues:

1) trying to avoid front-loading the cost of invoking sysinfo ( x-ref bug 1363586 ), and making it easy to test this configuration, I thought it'd make sense to have the gfx sanity test set a pref, which I'd key off to halve the result of https://searchfox.org/mozilla-central/rev/fc3d974254660b34638b2af9d5431618b191b233/gfx/thebes/gfxPlatform.cpp#3060 . Unfortunately, it seems that if I add this as a `Once` pref (which is how the frame_rate pref is configured) in gfxPrefs, the result is that we read the default value of the pref, rather than the user-set value, thus meaning the pref isn't honoured. I'm not sure what the best way to address this would be, and I'd appreciate pointers to the right point in gfx startup to add a conditional for this.

2) doing this will cause web-observable side-effects in animations, games, and maybe other things (video? apz/scrolling?). I noticed, as a somewhat bizarre example, that when trying to verify the frame rate was successfully adjusted and using https://www.testufo.com/ (search engine hit for 'frame rate test') that the page thinks the frame rate is so low that "something is wrong" ("Low Framerate. Try closing all apps and browser tabs."). So I'm wondering if that's something we're comfortable with, or if we'll need to do this in a more complicated way, where use of video or requestAnimationFrame or whatever "ups" the framerate for that tab (while it's active/selected).

Bas, are these points something you can help with?

Comment 1

[:Gijs (he/him)]

Attachment: PoC patch attempt

In case it's helpful, this is what I was trying, and what (AFAICT) doesn't seem to work.

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

The fingerprinting team would request that if RFP is enabled, you pretend you are not a low end machine, thanks!

Comment 3

[:Gijs (he/him)]

:rjesup, any chance you have ideas about this (see comment #0).

Comment 4

[:Gijs (he/him)]

Hm, seems based on bug 930793 and bug 1260070, maybe Olli has ideas about this - we could maybe also consider only temporarily lower frame rate while a toplevel doc is loading, or something like that? Olli, what's the current state of the "favor performance" code and/or does the idea in comment #0 sound crazy/sensible to you?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

We have other bugs open about trying to lowering frame rate.
Like https://bugzilla.mozilla.org/show_bug.cgi?id=1485100

Current "favor performance" is still just as crazy and broken as it has always been. Getting rid of it would be good.

But this bug sounds very much like something I've been thinking. Lower framerate at least in child process until the end of page load or max X seconds.

Comment 6

[Bas Schouten (:bas.schouten)]

(In reply to :Gijs (he/him) from comment #0)
> We're investigating lowering frame rate on low-end devices as a way to
> improve (perceived) performance.
> 
> Just setting the layout.frame_rate pref to 30, and running the raptor tp6
> page load tests on the 2018 perf reference device, I'm seeing 10%
> improvements across the board on load times compared with the default (-1)
> setting.
> 
> Trying to write a proof-of-concept patch, I've run into 2 issues:
> 
> 1) trying to avoid front-loading the cost of invoking sysinfo ( x-ref bug
> 1363586 ), and making it easy to test this configuration, I thought it'd
> make sense to have the gfx sanity test set a pref, which I'd key off to
> halve the result of
> https://searchfox.org/mozilla-central/rev/
> fc3d974254660b34638b2af9d5431618b191b233/gfx/thebes/gfxPlatform.cpp#3060 .
> Unfortunately, it seems that if I add this as a `Once` pref (which is how
> the frame_rate pref is configured) in gfxPrefs, the result is that we read
> the default value of the pref, rather than the user-set value, thus meaning
> the pref isn't honoured. I'm not sure what the best way to address this
> would be, and I'd appreciate pointers to the right point in gfx startup to
> add a conditional for this.
> 
> 2) doing this will cause web-observable side-effects in animations, games,
> and maybe other things (video? apz/scrolling?). I noticed, as a somewhat
> bizarre example, that when trying to verify the frame rate was successfully
> adjusted and using https://www.testufo.com/ (search engine hit for 'frame
> rate test') that the page thinks the frame rate is so low that "something is
> wrong" ("Low Framerate. Try closing all apps and browser tabs."). So I'm
> wondering if that's something we're comfortable with, or if we'll need to do
> this in a more complicated way, where use of video or requestAnimationFrame
> or whatever "ups" the framerate for that tab (while it's active/selected).
> 
> Bas, are these points something you can help with?

This part of our browser isn't exactly my specialty, Olli would probably know more about this. As far as the graphics portion, one thing you could try is not actually lowering the whole framerate, but just selectively skipping painting. That would cause less of an effect on web content and would probably still give a lot of the benefit, although if we'd be firing rAF at full speed we'd technically sorta be lying to the page. So I'm not sure that would fly spec-wise.

But yes, favor performance seems wrong to me. I also concur that lowering frame rate during page load is interesting. There's tradeoffs here though, a fast first paint may be important here for perceived performance, however I highly doubt with our page load times 1 frame is going to make a big difference, and most certainly the Throbber doesn't need to be painted at full framerate.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: slower_rate.diff

Something along this might not be totally crazy. Basically fall back to idle queue (which gets processed asap if there isn't anything else to process) or at most 2x of normal rate during page load


remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/feb3e6a56dacf78c04044e38e136a563ad856c99
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=feb3e6a56dacf78c04044e38e136a563ad856c99
remote: 
remote: It looks like this try push has talos jobs. Compare performance against a baseline revision:
remote:   https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=feb3e6a56dacf78c04044e38e136a563ad856c99

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

obviously that didn't do anything good. But I think it does reveal that processing .css files earlier is rather critical since that ends up triggering loads of other resources.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Olli Pettay [:smaug] (high review load) from comment #8)
> obviously that didn't do anything good. But I think it does reveal that
> processing .css files earlier is rather critical since that ends up
> triggering loads of other resources.

Absolutely.  They're a critical early node in the dependency tree.

60fps is probably mostly ok until we start to process incoming data (note: this will be before the first paint of content) - for example while waiting for DNS and initial html/css load, though even there on low-end machines you might contend over CPU pretty early.

Lower framerate (or more to the point, deprioritizing some frame painting when we have other things to do to saturate available cores) may well help during the "busy" part of page load. This is worth looking at, and perhaps doing some studies on.  1 frame difference will not be a problem; note however that users will start to scroll during "load", so the question also becomes "when do you end the slowdown"?  (end on first scroll or pageload done?)  Again, often later in load there are CPU idle times waiting for late resources; if we de-prioritize every other frame we might be able to hit 60fps at those times (and on higher-spec machines with lots of cores).

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Attachment: style_only_flushes.diff

Now trying a setup where style flushes still happen, but not reflows.
The idea is that whatever resources css files rely on, get loaded asap, but slow reflows or painting doesn't happen too often.

In theory something like this should improve performance, but let's see what
tryserver thinks.
remote: View your changes here:
remote:   https://hg.mozilla.org/try/rev/8f96017f49a4aab4b8111996be31b5ff85a740cf
remote:   https://hg.mozilla.org/try/rev/8ba69f23dcab24a222fbc4d0bb9bdd018e98d919
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ba69f23dcab24a222fbc4d0bb9bdd018e98d919
remote: 
remote: It looks like this try push has talos jobs. Compare performance against a baseline revision:
remote:   https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=8ba69f23dcab24a222fbc4d0bb9bdd018e98d919
remote: recorded changegroup in replication log in 0.063s


Perhaps I should use some other bug for this stuff...

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

I'll move my work to another bug. Let's keep this as low-end devices specific.

Comment 12

[:Gijs (he/him)]

Comment on attachment 9021261 [details] [diff] [review]
PoC patch attempt

Based on talking to Olli, mconley and others, and checking behavior of other browsers, it seems that for instance Edge already lowers frame rate when you turn on battery saver mode in windows 10. As a result, per some more discussion with mconley, I think I'd like to press ahead and write some experimental code that we can #ifdef for EARLY_BETA_OR_EARLIER so we can get some telemetry as to whether lowering framerates for (very) low-end machines makes sense. I'll make sure to include a possibility for users to override, and we can iterate on the specifics, but at a glance it seems worth at least pursuing to see if this improves stuff for folks.

The problem I was having when I filed this is still there though, it "doesn't work". It looks to me like when the pref is initially fetched through gfxPrefs, it always has the default value, and as a result we never enter the 'low end' mode. Looking for blame through gfxPrefs to find someone to help - kats, any chance you can have a quick look at this patch and let me know where I'm going wrong?

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 9021261 [details] [diff] [review]
PoC patch attempt

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

I think changing the pref to Live should solve the problem you were having

::: gfx/thebes/gfxPlatform.cpp
@@ +3057,5 @@
>  /* static */ int
>  gfxPlatform::GetDefaultFrameRate()
>  {
> +  printf("I think I am %s on a low-end machine\n", gfxPrefs::IsLowEndMachine() ? "" :"not");
> +  return gfxPrefs::IsLowEndMachine() ? 30 : 60;

Not sure this is the right place to do this, I think this is only used if we can't figure out hardware vsync rate or something.

::: gfx/thebes/gfxPrefs.h
@@ +739,5 @@
>    DECL_GFX_PREF(Live, "mousewheel.transaction.timeout",        MouseWheelTransactionTimeoutMs, int32_t, (int32_t)1500);
>  
>    DECL_GFX_PREF(Live, "nglayout.debug.widget_update_flashing", WidgetUpdateFlashing, bool, false);
>  
> +  DECL_GFX_PREF(Once, "performance.low_end_machine",           IsLowEndMachine, bool, false);

This needs to be "Live" instead of "Once". Once prefs are cached and don't reflect changes, but Live prefs do.

Comment 14

[:Gijs (he/him)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #13)
> Comment on attachment 9021261 [details] [diff] [review]
> PoC patch attempt
> 
> Review of attachment 9021261 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think changing the pref to Live should solve the problem you were having


So, I'm confused by this because the frame_rate pref is read correctly, on startup, if I modify it and restart, and it is applied, and it is also marked Once ( https://searchfox.org/mozilla-central/rev/a3894a4dcfb5d42f2e6eee6cb9faf7141879ef1a/gfx/thebes/gfxPrefs.h#702 ).

I guess the answer might be

> ::: gfx/thebes/gfxPlatform.cpp
> @@ +3057,5 @@
> >  /* static */ int
> >  gfxPlatform::GetDefaultFrameRate()
> >  {
> > +  printf("I think I am %s on a low-end machine\n", gfxPrefs::IsLowEndMachine() ? "" :"not");
> > +  return gfxPrefs::IsLowEndMachine() ? 30 : 60;
> 
> Not sure this is the right place to do this, I think this is only used if we
> can't figure out hardware vsync rate or something.

Hm, so it seems we force software vsync if the pref is set to a custom value, so that might explain why the pref is not taken into account, or something - that or its first read happens later than this pref now is. I guess this means that my initial experiment (which just changed the pref) might also be flawed in that the difference in behavior may in theory be a result of software vsync itself (though that seems somewhat implausible?). I'll verify some of this next week.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Some code reading the frame_rate pref doesn't use gfxPrefs and so will not be subject to the "Once" behaviour. Maybe that's a factor as well. https://searchfox.org/mozilla-central/rev/a3894a4dcfb5d42f2e6eee6cb9faf7141879ef1a/layout/base/nsRefreshDriver.cpp#1060

Comment 16

[:Gijs (he/him)]

Attachment: Bug 1503339 - try using a lower frame rate for low-end devices

Comment 17

[:Gijs (he/him)]

Attachment: Bug 1503339 - update frame rate at runtime, r?kats

Depends on D13260

Comment 18

[:Gijs (he/him)]

I have something working locally. However, it needs a restart for any changes to apply, which is problematic because:

1) we don't give users the best experience out of the box
2) it won't show up on raptor (which doesn't do an initial start/stop/restart cycle to set up the new profile)
3) it won't immediately react to changes e.g. to the resistFingerprinting pref.

To fix this, I'm trying to make our gfx platform use the new frame rate immediately, but I'm a bit out of my depth. My initial attempt stops updating the UI as soon as I change any of the observed prefs, so I'm definitely doing something wrong - just not sure where to look. I asked Nical to have a look, even though the patch is not yet finished.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/195290355ea5
try using a lower frame rate for low-end devices r=kats,mconley
https://hg.mozilla.org/integration/autoland/rev/d4d1f7ec6966
update frame rate at runtime, r=kats

Comment 20

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/195290355ea5
https://hg.mozilla.org/mozilla-central/rev/d4d1f7ec6966

Comment 21

[:Gijs (he/him)]

(In reply to Markus Stange [:mstange] (away until Jan 8) from bug 1485100 comment #4)
> My main concern here is scrolling responsiveness: If we only composite at
> 30fps, scrolling will be sluggish. It would be much better if we could
> somehow dial down our main thread refresh tick frame rate, and maybe our
> OMTA animation sampling frame rate, without locking the compositing frame
> rate and affecting scrolling.

Wondering if this is also a concern with the attached patch, and if it'd be helpful to file a follow-up bug for this, ideally with a rough idea of how to implement such a more specific change?

Comment 22

[Markus Stange [:mstange]]

(In reply to :Gijs (he/him) from comment #21)

(In reply to Markus Stange [:mstange] (away until Jan 8) from bug 1485100
comment #4)

My main concern here is scrolling responsiveness: If we only composite at
30fps, scrolling will be sluggish. It would be much better if we could
somehow dial down our main thread refresh tick frame rate, and maybe our
OMTA animation sampling frame rate, without locking the compositing frame
rate and affecting scrolling.

Wondering if this is also a concern with the attached patch,

I think it is: The patch is forcing software vsync with a refresh rate of 30, and vsync is what drives both the layout refresh driver and the compositor. If the compositor only composites at 30fps, this affects scrolling. I've filed bug 1519241 about this.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

If I change RefreshDriver scheduling, I'm starting to see
Assertion failure: SingletonExists(), at z:/build/build/src/gfx/thebes/gfxPrefs.h:765
https://searchfox.org/mozilla-central/rev/69d3c6c61dca9a41f14797cd9924733289238d1a/gfx/thebes/gfxPrefs.h#765 on Mn and MnH tests

Comment 24

[:Gijs (he/him)]

(In reply to Olli Pettay [:smaug] (massive needinfo queue, ping on IRC on anything urgent) from comment #23)

If I change RefreshDriver scheduling, I'm starting to see
Assertion failure: SingletonExists(), at z:/build/build/src/gfx/thebes/gfxPrefs.h:765
https://searchfox.org/mozilla-central/rev/69d3c6c61dca9a41f14797cd9924733289238d1a/gfx/thebes/gfxPrefs.h#765 on Mn and MnH tests

Can you link to an occurrence?

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

Sorry, lost the tryserver link for the assertion failure.