1053275 - ServiceWorkers must not be blocked from running by 20 worker thread limitation

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently we queue up workers if there are already 20 outstanding workers for a domain.  ServiceWorkers need to be exempted from this limitation since navigation can break if they are suspended.  Also, if its that easy to prevent a ServiceWorker from running, then they will not be reliable enough to use for their intended purpose in practice.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Punting to v2 since 20 workers on a website seems unlikely to happen at this point on the web considering use of workers is fairly low and serviceworkers is non-existent.

Comment 2

[Lars T Hansen [:lth]]

(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #1)
> Punting to v2 since 20 workers on a website seems unlikely to happen at this
> point on the web considering use of workers is fairly low and serviceworkers
> is non-existent.

Workers are the new threads: bug 1131757.

Comment 3

[Andrew Overholt [:overholt]]

This is affecting mochitests; one option brought up is to have a pref that removes the limit.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I believe this is an issue for mochitests because each test ends up registering its own scope.  Real world apps will only experience the same thing if they register a ton of scopes that get triggered at the same time.

Exempting mochitests from the limit would be a nice short term solution.  Maybe we can get telemetry to see how many scopes real world sites are registering in the wild.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

My plan is:
1) Add a testing only pref to exempt the limit for now - dom.serviceWorkers.exemptFromPerDomainMax
2) Add a telemetry probe and console warnings to RuntimeService if ServiceWorkers hit limits - Filed bug 1139513. Based on stats we can either bump up the per domain limit, have a separate limit exclusively for SWs or make them unbounded.

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Exempt ServiceWorkers from per domain thread limits

Adds the pref and enables it in all tests.

Comment 7

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=77a9f6b0630d

Comment 8

[(no longer active)]

Can we get someone else to review this?  At this point, any new SW test landing means that some other test starts to fail too often, hindering landing new stuff.

Comment 9

[Andrew Overholt [:overholt]]

Comment on attachment 8572779 [details] [diff] [review]
Exempt ServiceWorkers from per domain thread limits

Jonas told me he could take this review while Ben's on PTO.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8572779 [details] [diff] [review]
Exempt ServiceWorkers from per domain thread limits

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

r=me assuming that you're sure you're always on main thread.

::: dom/workers/RuntimeService.cpp
@@ +1372,5 @@
>    }
>  
> +  bool exemptFromPerDomainMax = false;
> +  if (aWorkerPrivate->IsServiceWorker()) {
> +    exemptFromPerDomainMax = Preferences::GetBool("dom.serviceWorkers.exemptFromPerDomainMax",

Are you sure this will only be on the main thread? Is that because SWs never have a parent thread? At least add an assertion that you're on the main thread.

Comment 11

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/36c32d9b014e

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/36c32d9b014e