Enable budget throttling by default

RESOLVED FIXED in Firefox 58

Status

()

P2
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: farre, Assigned: farre)

Tracking

(Depends on: 1 bug, {dev-doc-complete})

unspecified
mozilla58
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Assignee: nobody → afarre
Depends on: 1362322
(Assignee)

Updated

a year ago
Depends on: 1378123
(Assignee)

Updated

a year ago
Depends on: 1378124
(Assignee)

Updated

a year ago
Depends on: 1378125
(Assignee)

Updated

a year ago
Depends on: 1378356
(Assignee)

Updated

a year ago
Depends on: 1378402
(Assignee)

Comment 1

a year ago
As expected there are loads of oranges if this is turned on:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7161c02e0c11902e3084c5b31d60959454b70bd8&selectedJob=118414848

Haven't checked all (there are 500+ failures), but those I did check were of the test timed out variety.
(Assignee)

Updated

a year ago
Depends on: 1384917
(Assignee)

Updated

a year ago
Depends on: 1384924
(Assignee)

Updated

a year ago
Depends on: 1385238
(Assignee)

Updated

a year ago
Depends on: 1391602
(Assignee)

Updated

a year ago
Depends on: 1393056
(Assignee)

Updated

a year ago
Depends on: 1393359
(Assignee)

Updated

a year ago
Depends on: 1393764
(P2 only because we're going to try to ship it soon but it's not a super-critical crasher or something like that :)
Priority: -- → P2
Are there plans to use different timeout values on Android given that currently background timeouts are much more heavily throttled there? (https://dxr.mozilla.org/mozilla-central/rev/c97190c389c4cfef20fe55b4bacade95a36ae6ef/mobile/android/app/mobile.js#741)
See Also: → bug 1404042
(Assignee)

Comment 6

a year ago
Created attachment 8916574 [details] [diff] [review]
0001-Bug-1377766-Enable-budget-throttling-by-default.-r-b.patch
Attachment #8916574 - Flags: review?(bkelly)
Attachment #8916574 - Flags: review?(bkelly) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 7

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbcd58f04e4f
Enable budget throttling by default. r=bkelly
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dbcd58f04e4f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Updated

a year ago
Keywords: dev-doc-needed

Comment 9

a year ago
Key word "perf"?
I've started documenting this by adding a section to the Page Visibility API landing page to explain the policies we have in place to mitigate perceived background performance problems:

https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API#Policies_in_place_to_aid_background_page_performance

I thought this seemed like OK place to put it.

To help with this, can you:

1. Let me know if you think this place is ok.
2. Let me know what the effects of Budget based background timeout throttling are — do we follow the same policy as Chrome, e.g. https://developers.google.com/web/updates/2017/03/background_tabs#budget-based_background_timer_throttling , or something else?

Once I know this, I'll fill out that section, and add a note to the Fx58 rel notes.

Thanks!
Flags: needinfo?(afarre)
(Assignee)

Comment 11

a year ago
It looks like the perfect place. 

We follow a /similar/ policy, matching all bullets from https://developers.google.com/web/updates/2017/03/background_tabs#budget-based_background_timer_throttling we behave as follows:

* Windows in each background tab in the background has a time budget in milliseconds

Not sure if this differs in that Chrome has per tab where we have per window. Our budget is in milliseconds with a max and a min value +50 ms, -150 ms respectively.

* Windows are subjected to throttling after 30 seconds, mainly because we re-use the throttling delay from Bug 1355311.

This has been manually tested, but is a bit hand-wavey.

* Timer tasks are indeed only allowed when the budget is non-negative.

* When a timer has executed, the execution time is deducted from the budget from the window where the timeout gets called from.

* Budget regenerates at the same rate, 10 ms per second. 

The exceptions from throttling are the same. We also add IndexedDB to the list of exceptions, mainly due to https://bugs.chromium.org/p/chromium/issues/detail?id=675372, but now when checking Chrome seems to throttle even when there are active IndexedDB transactions. Also, we differ from Chrome in that Chrome stops throttling pages with audio, and will not start throttling again when audio stops playing.

For the curious here's the toy I wrote and put here: https://farre.github.io/web-tools/load/ The UI is a bit messy, but just hit the 'Run Test' button and wait to see the throttling behaviour of the UA.
Flags: needinfo?(afarre)
(Assignee)

Comment 12

a year ago
Sorry, that Chromium issue regarding IndexedDB is not relevant. Regardless, I'm fairly certain that Chrome also used to limit throttling of IndexedDB to once per second, but not with budget. Will check this some more tomorrow.

Comment 13

a year ago
(In reply to Worcester12345 from comment #9)
> Key word "perf"?

So this wasn't a performance fixing bug then?
Cool, thanks a lot for the notes Andreas! I have filled in those details on the page mentioned:

I have also added a note to the Fx 58 rel notes:

https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API#Policies_in_place_to_aid_background_page_performance

https://developer.mozilla.org/en-US/Firefox/Releases/58#DOM

Please let me know if you think the section needs any more information, or fixes.
Flags: needinfo?(afarre)
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 15

a year ago
Looks excellent!
Flags: needinfo?(afarre)
You need to log in before you can comment on or make changes to this bug.