Proposal: Optimized No-Fetch Service Workers

[fatlotus]

Hey all-

In Blink, main resource fetch with a cold Service Worker causes sadness (median Android/470 and Linux/210 ms), even if the worker doesn't intercept requests. To reduce this a bit, I've been looking into a) detecting Service Workers without fetch event handlers, and b) immediately falling back to the network. Unfortunately, as things stand today, fetch event handlers can be registered at any point, making (a) rather difficult.

A discussion in #195 about install and activate ended up with a devtools warning, but it seems (naively) like that has less of a performance impact. Does this warrant more harsh restrictions in fetch? We're mainly worried about i) non-deterministic registration for a given SW version, and ii) registration after the end of install.

(I've written a basic Design Document for this feature, if you're curious.)

[annevk]

I think the answer Mozilla has been given whenever we brought this up was that you should not have service workers without fetch. :-/

[jakearchibald]

@fatlotus are we seeing many SWs without a fetch event? I know there's a few at the moment to trigger Chrome's app install banner, but we're going to start tightening the heuristics here, we only want to promote add-to-homescreen for sites that have some offline capability.

Listeners should be deterministically added to the SW global during initial execution. Both the Math.random case and adding listeners later are antipatterns.

+@slightlyoff

[annevk]

Observing listeners is also an established anti-pattern however, as we mentioned, and a much older one at that.

[mfalken]

We're seeing some sites that use SW just for Push Notifications. Pinterest is one.

[annevk]

Right, that's exactly the kind of feedback Mozilla gave for why we should not treat fetch as a builtin.

[jakearchibald]

Right, and Pinterest will get the poor user experience of "user clicks notification, gets generic browser offline page".

That said, the workarounds in the doc seem pretty sensible.

[wanderview]

I think many of these sites care a lot about push on the desktop where they couldn't reasonably get it in the past. Offline for mobile is more of a "nice to have" since they already have native apps there.

[kinu]

From implementation & optimization pov it'd be definitely good if we can clearly say listeners should be added to the SW global during initialization somewhere in the spec. On the other hand if we decide not to spec it out (e.g. for not promoting such usage) there seems to be still a way to optimize the implementation without affecting the Web-facing behavior, though the code would become less clean.

[jungkees]

Currently we settled on #225 (comment) but people didn't want to override addEventListener to record the registered event types.

I'd like to try to add the list of event types to handle on the service worker's environment settings object created in step 5 of Run Service Worker algorithm. And set its value after step 14 like the following:

14. Jump to the script's code entry-point, and let that run until it either returns...
15. Set settingsObject's _list of event types to handle_ to the set of event types retrieved from the _associated list of event listeners_ of settingsObject's global object.
16. Run the responsible event loop specified by settingsObject until it is destroyed.

As step 15 runs before the event loop starts running, the list of event types to handle will retain only the event types that have been added during the first script eval.

And in step 14 of Handle Fetch algorithm, we can do:
If registration's active worker's environment settings object's list of event types to handle doesn't contain fetch, return null (i.e, fall back to network).

[jungkees]

Ah I think we'll have to check the list of event types to handle even before a service worker starts running, so the property itself should be an internal slot of a service worker instead of the environment settings object. Anyway, the idea is just get the set of event types from the initial script eval and use that to check whether we should not actually run a worker but rather just fall back to network.

[jungkees]

Added the set of event types to handle internal slot and made the Handle Fetch use it to enforce an early return: f8d94f5. Also added a note about showing a warning for non-deterministically added listeners.

[wanderview]

gecko impl bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1181127

[mfalken]

Consensus at the Service Worker informal working session F2F was to not observe listeners, and instead add a method like disableFetch() to be called in the install event handler.

Reasoning:

• observing listeners would break the invariant that addEventListener has no side effects, potentially making SW spec's event model diverge from DOM spec's.
• having an explicit listener list is an alternative; however fetch event is the only one you need it for as the others are already opt-in.
• an explicit listener enableFetch() would break current sites
• therefore, disableFetch() was chosen

[kinu]

Let me make sure. If the impl could make the optimization without changing the addEventListener behavior (though supporting all corner cases is hard) it should be fine / up to UA, right?

I kinda agree that we probably wouldn't want to spec out something that affects generic addEventListener behavior.

[annevk]

@kinu sure, non-observable optimizations are fine.

[wanderview]

I think we felt spec'ing the optimization was necessary previously because it is observable. If script does setTimeout(addEventListener.bind(self, handlerFunc)) then the optimization prevents the late addEventListener call from working. This is definitely observable.

Anyway, the explicit API avoids this.

[kinu]

@wanderview Yeah there're several tricky cases that make non-observable optimization less (if not) practical. For the particular setTimeout case you wrote, though, it looks technically the impl could just flip the optimization flag when it's called (or UA may just kill the SW before the timer fires regardless of the optimization)? I agree that there could be various more tricky cases.

Anyway, the explicit API avoids this.

Yup.

[aykevl]

With this proposal, it's impossible to use ServiceWorker for other tasks and just forget about caching (while keeping good performance). You have to do one of these two things:

• Properly implement the fetch event (with caching, full offline capabilities, or whatever).
• Call disableFetch() so the browser knows it can go straight to the network without firing a fetch event first.

Personally, I dislike the opt-out nature of this proposal. It's too easy to forget if you don't want to use offline capabilities, and adds more boilerplate code. It's also counter-intuitive to me.

Many other capabilities have been mentioned for service workers, that are not dependent on offline capabilities, like push notifications, background sync, and geolocations. We don't know yet where service workers will actually be used for in 5 years.

Additional reasons why I think opt-out is a bad idea:

• It isn't currently visible that there is a performance problem when forgetting disableFetch() in applications that don't care about offline capabilities. Maybe browsers will eventually have to implement some form of fetch event detection for badly written service workers. Though with disableFetch() I think it's a good idea to throw a warning in the developer console if no fetch handlers are registered when disableFetch() has not been called by the end of the install event.
• Some applications are inherently dependent on the network. A feature like background sync can't work without network access.

Note that, again, service workers are very new so everyone using them now should expect things can break with every new browser release. Consider ServiceWorkerMessageEvent which suddenly moves from window to navigator.serviceWorker (offtopic: good move!). Now displaying warnings about how the fetch event is going to change in a few versions and requiring to opt-in to the events doesn't seem like a big problem to me. The longer you wait with such changes, the harder it is to change.

(This is my opinion on the matter, as someone who doesn't (yet) work professionally as web developer.)

[kinu]

@aykevl I don't think that is true, developers can appropriately specify the scope attribute in the Registration Options when they register a SW to say what URLs the site wants their SW to take care of for fetch events. That's one of the most basic fundamental of how SW is designed to work.

Also a feature like background sync is a way to give more flexible options for a site to choose when it talks to the server, it can't work without network access but it'd be nearly useless if its SW doesn't handle fetch events when network access is not available.

[kinu]

(Well I just learned that there's also discussion on the concept of scope.)

[wanderview]

Will the navigator.serviceWorker.controlled attribute still reference the service worker matching the page's scope even when disableFetch() has been called?

[jakearchibald]

From IRC:

My hunch would be that the pages are still controlled. It allows you to defer idb upgrades to an activate event of a sw, avoiding multiple tabs with different versions. Also makes sense with clients.matchAll.

[slightlyoff]

Pages are still controlled by scope if you disableFetch. I think we've covered all the bases here. Just need to spec disableFetch()

[mfalken]

Implementor feedback from Chrome: We don't really plan to implement disableFetch(). It's an irreversible change if devs start using it, and I think we're not yet convinced it's needed. We're OK taking the perf hit for now and remain hopeful that a non-observable optimization is possible.

[jakearchibald]

F2F resolution: We're going to observe addEventListener and optimise based on that.

addEventListener should throw if it's called after the initial execution of the script.

[mfalken]

Cool! Just to make sure, initial execution of the script refers to the very first time the SW runs (before it's installed) or every time the SW is run?

The current spec seems to observe event types every time in Run Service Worker, which wouldn't allow for this optimization. We want to persist a bit on the installed script indicating whether there's a fetch event handler. So next time you go to the website, we can decide whether the SW needs to run.

[mkruisselbrink]

For the optimization initial execution indeed means to record which event listeners have been registered for at the end of the very first time the SW runs.

Furthermore every time the SW runs, after its initial execution has completed (for that run) addEventListener on the global scope should throw (but the list of event listeners is not updated after the first initial execution). And this is not just about fetch, but would apply to every event in ServiceWorkerGlobalScope.

[jungkees]

We're going to observe addEventListener and optimise based on that.

Relevant changes for this and enforcing the early return to all the functional events have been addressed: b80ba58

addEventListener should throw if it's called after the initial execution of the script.

I'll make a PR to DOM spec for this.

[jungkees]

Closed by b80ba58 and whatwg/dom#155