Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guidance on background sync and shared workers #70

Closed
jrburke opened this issue Apr 1, 2015 · 19 comments
Closed

Guidance on background sync and shared workers #70

jrburke opened this issue Apr 1, 2015 · 19 comments

Comments

@jrburke
Copy link

jrburke commented Apr 1, 2015

Andrew Overholt of Mozilla suggested I post here about use cases that come up in the context of Firefox OS and service workers that want to do background sync. It may help with the use case document.

Context

The email app on Firefox OS. It wants to do background syncs every so often when there is connectivity available. The user indicates a rough time interval (5 mins, 10 minutes, 15 minutes, 30 minutes, 1 hour). The sync interval does not have to be exact, just approximate to those intervals, and only if there is connectivity available.

Background sync is used instead of something like a push API-backed server because it is a generic email application that can connect to IMAP/ActiveSync/POP3 servers that likely do not have support for push.

Firefox OS apps can have multiple browser windows opened per app, so the email app would want to use a shared worker to do the network and IndexedDB work for syncing and for handling serving slices of the email data to the browser windows. We just what one entity responsible for the network connections and data services, and holding the business logic related to the data models.

Issue

This gets a bit tricky though when considering a service worker with a background sync entry point:

  • The sync could come in and there could be no open email windows, so the shared worker may not be spun up yet.
  • Or, the background sync could come in while there is an open email window, and the shared worker is available.

However, service workers spinning up shared workers did not seem to be allowed, if I read the old issues list for service workers correctly.

So guidance on this use case is appreciated. Here are some things that were discussed when talking about the issue within our group:

Possibilities

Talk to shared worker: Maybe there is a way for the background sync entry point to communicate to the shared worker, and start up the shared worker if not running, and wait for it to complete its network and database work before signaling the sync is complete.

However, it is my understanding that this is not allowed in a service worker at the moment.

Service worker as the backend: Use the service worker as the shared worker, so embedding the code for the sync work in the service workers, and the user visible browser windows for the app would talk to it for the data slices.

However, we do not want to burden the service worker with more work, where it might interfere or slow down the service worker handling its basic fetch duties. Plus, the service worker seemed volatile, and not really designed to be more long-lived for use by user visible browser windows for business logic/data services for the UI.

openWindow: Perhaps the service worker could use openWindow to open a non-visible browser window that would then talk to/spin up the shared worker.

However, it is unclear if those windows would be user visible or not. In this case we would not want that window visible as it is just a pipeline to spin up the shared worker, and that window needs to be destroyed at some point. It seems like a better expression of intention to just spin up the shared worker directly, as something that is not visible and just related to data processing.

@annevk
Copy link
Contributor

annevk commented Apr 1, 2015

If the service worker only does asynchronous work, I think it should be able to be the backend. But I also don't see why we'd disallow opening workers from a service worker. They might be shortlived as well, but I'm not sure why we'd explicitly disallow running them.

@wanderview
Copy link

@annevk Step 10 in the SharedWorker spec says:

Add to worker global scope's list of the worker's Documents the Document objects in docs.

We don't have a Document in ServiceWorker. That would need to be adjusted somehow.

In general, though, I think the ServiceWorker is the "SharedWorker without a document" concept. Maybe we just need a way for them to attach to a separate ServiceWorker script (at a different scope?).

@wanderview
Copy link

@jrburke In regards to "the service worker seemed volatile" issue, I believe there are plans to implement a waitUntil(promise) function that will explicitly hold the ServiceWorker alive.

I agree, though, if you are doing sync work that should probably be on a separate worker.

@wanderview
Copy link

@jrburke Can you have a separate ServiceWorker scripts registered to handle background sync and fetch events? That way work done in the first SW would not delay handling a fetch event.

@jakearchibald
Copy link
Collaborator

Agree with @annevk here. @jrburke - are you expecting to do any heavy synchronous execution here?

I'm really worried about battery usage here so periodic sync should be seen as "beneficial" rather than "critical". Doesn't email fall more into "critical"?

How will you communicate with the IMAP/ActiveSync/POP3 servers? Do they have CORS-enabled HTTPS endpoints?

Good use-case though! Same applies for decentralised RSS reader, although I guess the CORS+HTTPS requirement will bite you there.

@jrburke
Copy link
Author

jrburke commented Apr 1, 2015

Something I left out of the use case that might also help illustrate the issue more:

In addition to the background sync, the user has the option in the mail UI to trigger a sync via a button. Since multiple user-visible browser windows could trigger a sync, as well as the background sync, we only want one entity doing the actual sync work, since it can coordinate only doing one sync at a time, to accurately communicate sync positions to servers, and discarding others that perhaps are triggered during the same time.

This seemed to point using a shared worker for that sync work.

It sounds like the advice might be "use a separate service worker from the one that handles URL fetches", because perhaps spinning up other service workers is allowed from a service worker, and that also works in browser windows too. So, using the service worker as a shared worker, but since service workers are not bound by the shared worker "add the document" part of the shared worker spec, it side steps the shared worker issue?

My concern with that approach: right now the (plain) worker in email handles the server syncing but also IDB and JS model work around the IDB data, so the browser windows use the worker for data/model services for the UI, and for memory/code co-location concerns I would ideally like to keep that logic in the worker that does the server synchronizing vs spinning up a specialized worker just for the sync part.

If there are ways to make sure the service worker that had the sync code could stay up and available for the data actions that seems to be ideal. Perhaps it is as simple as the browser window holding on to the reference for that service worker instance for as long as the browser window is up?

@jakearchibald on your questions:

Battery usage is a concern for us too if you mean the cost of doing a polling action vs relying on something like a push api where the app is only woken up when there is new data. However, given the sensitivity of email credentials and email contents, we do not want to, for example, run a generic email server proxy to do the sync checking/push API work as it would be a very tempting target for hackers.

As to "beneficial" vs "critical", I am not familiar if those terms mean specific things in this context. Syncing for the email app is best effort given circumstances: not wanting to run a server piece/proxy, the user understanding the variability of network on mobile. The "last sync" time is shown in the UI, and the user can trigger a sync from the UI if they want to be assured of the most recent data visible, which I believe helps with bridging gaps in any automated syncing.

Currently we use mozTCPSocket, a Firefox OS-specific API only visible to certified apps, to talk to the servers, since this is a generic email client that can connect to any IMAP/ActiveSync/POP3 server.

I think those details are more side notes though. The general use case is:

We have data work to do that is possible to trigger from user visible UI or the background sync entry point, and we only want one entity doing that work to enforce data coherency and to use the network efficiently.

That entity could also be providing data services to user visible UI outside of the basic data synchronization. This is done for code co-location and memory concerns: many of the JS data objects are used for the sync-to-IDB storage step and browser UI, and some APIs are shared with actions triggered from the browser UI (moving/deleting messages, starring emails for example). Ideally loading that code again in a separate dedicated service worker just for syncing could be avoided because of extra memory and code loading configuration concerns.

Sounds like the guidance is:

Use a separate service worker from the one use for URL fetches. That separate service worker can handle the background sync entry point and browser windows can spin it up and hold on to it for data services, communicating with it using normal worker communication methods. As long as the browser windows hold on to that service worker reference, it will stay up and act like a shared worker in that case.

If that is the case, feel free to close this issue.

@wanderview
Copy link

Holding the ServiceWorker ref does not keep the worker alive. You will need something like w3c/ServiceWorker#669.

Also, its unclear to me if the current API gives you an easy way to spin up the ServiceWorker if its not already running in the "manual sync button" case. We may need some new API here.

@jungkees
Copy link
Collaborator

jungkees commented Apr 2, 2015

Maybe it's a good use case of navigator.connect() with the concept of stash MessagePorts proposed by @mkruisselbrink.

@jrburke I'd imagine a setting where you register two separate SWs: (A) the sync worker and (B) the business logic worker. (B) replaces the existing shared worker.

Email clients (documents) connect to (B) and do the channel messaging as they've done with the shared worker. In this setting, the clients and (B) can communicate regardless of (B)'s lifetime.

(A) also can connect to (B), and they can do the channel messaging regardless of the lifetime of both ends.

Upon (A)'s sync request, (B) can run update and get the latest data ready without the existence of clients.

// from a Window to (B)
navigator.serviceWorker.ready.then(registration => {
  registration.getStashedPorts('toB')
    .then(port => { port.postMessage('doSync'); });
});

// from (A) to (B)
self.addEventListener('sync', function(e) {
  self.registration.getStashedPorts('toB')
    .then(port => { port.postMessage('doSync'); });
});

(B) running sync operations in message events' waitUntil(p) seems not worse than running them in a shared worker I suppose.

And how about introducing a SW's persistency mode like the following?

// makes the service worker run as long as *any* client is being loaded or running.
navigator.serviceWorker.register("sw.js", { persistency: "max-binding" });

/cc @mkruisselbrink, @jakeleichtling, @slightlyoff, @reillyeon, @KenjiBaheux

@jakearchibald
Copy link
Collaborator

In addition to the background sync, the user has the option in the mail UI to trigger a sync via a button.

I suggested .syncNow over at #72 (comment) - this is another good use-case for it.

but since service workers are not bound by the shared worker "add the document" part of the shared worker spec

The spec only uses documents for life cycle, they should be easily replaceable with something more generic. A some kind of worker within the SW is a way simpler solution than managing two serviceworker registrations. Even as an "in the meantime" hack, I'd sooner go for splitting the work into chunks with setTimeout than a separate registration.

Battery usage is a concern for us too if you mean the cost of doing a polling action vs relying on something like a push api where the app is only woken up when there is new data.

No, I'm more concerned about the amount of pure JavaScript processing going on. If your code is doing simple network fetches and adding to a cache, that's all async, you don't need another worker for this. Your need for another thread makes it sound like you're doing heavy synchronous processing in JS, and that makes me worry about battery.

Sounds like the guidance is: Use a separate service worker from the one use for URL fetches.

I'd advise against that, but it would work.

@jakearchibald
Copy link
Collaborator

Using background sync for CPU intensive processing (the kind that would lock up the ServiceWorker thread) sounds like an antipattern. IMO, sync should be for little more than a network fetch & storage, maybe showing a notification if something new has been found.

If that data requires heavy processing, that should be deferred to the next opening of the app, where extended CPU use is more reasonable.

@wanderview
Copy link

If that data requires heavy processing, that should be deferred to the next opening of the app, where extended CPU use is more reasonable.

Doesn't this disadvantage web apps then compared to native? Are native apps discouraged from pre-processing sync'd data ahead of the app opening? Seems like we're forcing UX jank with this policy.

@jrburke
Copy link
Author

jrburke commented Apr 2, 2015

It seems like it would just be nice to change shared workers to track things other than documents to keep them alive. So if there are pointers on how to accomplish that, it seems like it will solve my issue.

Then the service worker onsync listener would:

  • spin up the shared worker (or just get a handle to it if already running).
  • message the shared worker to do the sync.
  • Use waitUntil in the the onsync listener to make sure the shared worker stays up while it does its work
  • when the shared worker messages the service worker to indicate it is done, the service worker clears its reference to the shared worker and completes the waitUntil flow to indicate syncing is done.

I am not sure navigator.connect() helps directly with this issue, as the domain that registers the service worker will also create the shared worker, and my assumption is that we could use normal worker-to-worker communication in that scenario.

I agree with @jakearchibald that service worker work should be kept light. However, for this sync case, deeper work needs to be done, and it would be nice for the service worker to just hand off to a shared worker in that case. That helps keep the division of responsibilities clearer.

Some notes on not wanting to delay processing of the sync in this particular email case:

  • We need to load up a bunch of JS to implement and respond to the mail protocols, and dealing with all of that feels heavier weight than the actual data processing. But we will have some data processing for sure.
  • The user wants email background sync because they also want to get immediate notifications on the phone of new mail or at least part of the new mail. The data from the server needs to be processed at the time of the sync so that we can for example generate a snippet from the email body to show in the notification message.
  • Getting to @wanderview's point, the work needs to be done at some point, and we could even pre-seed some caches used by the service worker so that for next app launch we could show the new messages in the "index.html" fetch, for fast perceived startup.

I can see cases where immediate processing may not need to be done for a sync, for things like ebook syncing, but for email, the data processing desires seem more immediate. So it seems best if we could just delegate to a shared worker then in that case.

@slightlyoff
Copy link
Collaborator

So it seems we need a few things here (OH HAI @jrburke!!!):

  • waitUntil() could plausible handle some of this work, in particular, you could imagine a situation where you do some fetching work, save some stuff, and then process it on a sub-worker (thread) to keep the main SW thread responsive but guard the whole thing in a Mega Promise (TM). ISTM that the discussion there is the amount of running time that we give to background syncs (and the visibility that apps have into that timing).
  • some sort of "independent liveness" for Shared Workers? E.g., what if Shared Workers could outlive their hosts based on something like waitUntil()? Seems ismorphic to the first point (in terms of execution constraints), but perhaps would simplify the programming model for folks who want to do work like this in Shared Workers and not think about how to split it between SWs/Shared Workers?

@annevk
Copy link
Contributor

annevk commented Apr 3, 2015

A shared worker per specification can already survive navigation. But I don't think it should be able to last long without a browsing context as that would have serious privacy implications. However, opening one from a service worker we should seek to make work somehow. There's no reason why service worker threads, even though they are shortlived, should be banned from spawning parallel threads (that take the lifetime of the service worker if tied to nothing else).

@jakearchibald
Copy link
Collaborator

Will reply to other point later, but was chatting to some Android
engineers. Seems their liberal stance on background sync is part of the
battery problem Android has, so I don't think we should simply copy native.

Also, just because work is on a different thread, doesn't mean it's free.
Doing processing work in background sync is attractive because you're not
going to slow down or jank your app, but you may affect someone else's.

On Fri, 3 Apr 2015 11:49 Anne van Kesteren notifications@github.com wrote:

A shared worker per specification can already survive navigation. But I
don't think it should be able to last long without a browsing context as
that would have serious privacy implications. However, opening one from a
service worker we should seek to make work somehow. There's no reason why
service worker threads, even though they are shortlived, should be banned
from spawning parallel threads (that take the lifetime of the service
worker if tied to nothing else).


Reply to this email directly or view it on GitHub
#70 (comment)
.

@jakearchibald
Copy link
Collaborator

I'm still trying to get a handle on the predicted hot points of this script.

Is the worry that the amount of code needed to communicate with various email servers will negatively impact the spin-up time of the ServiceWorker for unrelated actions? If so, then making (shared)workers work within a SW seems like the best answer, although would this be a problem with things like (code caching)[http://blog.chromium.org/2015/03/new-javascript-techniques-for-rapid.html]?

If the worry is pure JS-processing after the data has been fetched, then I think we need another kind of sync for that (#77).

@jrburke
Copy link
Author

jrburke commented Apr 13, 2015

The main issue is that we have a shared worker that handles the data layer for the email app: it converts data from network calls into JS objects suitable for the UI to consume and stores that data in IndexedDB.

We only want one thing doing that work, to properly manage/constrain network connections, provide a coherent data model, and isolate code for developer understanding. So we want the shared worker to do that.

Even without backgroundsync, we want this structure to support the multiple browser windows that can be open to the email app. Email sync can be triggered manually in the UI view, and that sync would go to the shared worker.

So for this use case, I believe it is enough to focus on enabling the service worker to spin up a shared worker, and the shared worker to treat a service worker (instead of just documents) as something that can participate in the ref counting that keeps the shared worker alive.

If there is another place to lobby for that capability instead of here, feel free to point me to that place. If it is helpful, I can point to this thread as motivation for that capability.

I did not follow how that capability would cause a problem for code caching. I do not believe adding a second "processingsync" entry point for a service worker helps this case, but I will comment more in #77 on that, and maybe we can use that issue to discuss hot points.

@jakearchibald
Copy link
Collaborator

Ok, I get it now, thanks for dragging me through!

Completely agree, allowing the ServiceWorker to create a SharedWorker and contribute to the refcount is the right answer here.

This should be closed in favour of w3c/ServiceWorker#678

@jrburke
Copy link
Author

jrburke commented Apr 14, 2015

This use case has been explored, and it looks like w3c/ServiceWorker#678 will be enough to support it, closing this ticket.

@jrburke jrburke closed this as completed Apr 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants