If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[Messages][NG] Use SimpleOfflineCache to cache all app resources

RESOLVED WONTFIX

Status

Firefox OS
Gaia::SMS
RESOLVED WONTFIX
2 years ago
7 months ago

People

(Reporter: azasypkin, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Since we're gradually moving to the hosted model, we'll likely want to leverage Service Worker "offline" cache to store all app resources.
Blocks: 1172906
(Reporter)

Comment 1

2 years ago
Created attachment 8617911 [details] [review]
GitHub brach URL (wip)

Here is just WIP patch that uses SimpleOfflineCache from ServiceWorkerWare (SWW). It's based on patch from bug 1162028 and works only in browser (http://sms.gaiamobile.org:8080/view/inbox).

It doesn't work for app:// neither in browser nor on device. :ferjm is looking into this.


Hey Francisco I have few questions for you:

1. How are you going to distribute SWW lib? Bower, npm, gaia/shared? Since Threads.js lib is a bower component we have shell script in Messages to automatically update it, maybe we need something like this for SWW lib as well.

2. What do you think about https://github.com/gaia-components/serviceworkerware/issues/22 (sorry GitHub doesn't have ni? :)).

Thanks!
Flags: needinfo?(francisco)
(Reporter)

Updated

2 years ago
Depends on: 1173539
Hi :azasypkin,

great questions:

- We can setup a bower package, I think is the easy part, we have the dist files committed with that in  mind.

- Definitely the impact in performance is huge. So we will go with your suggestion.

When do you need this changes, ASAP I guess?
Flags: needinfo?(francisco) → needinfo?(azasypkin)
(Reporter)

Comment 3

2 years ago
Hey!

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #2)
> Hi :azasypkin,
> 
> great questions:
> 
> - We can setup a bower package, I think is the easy part, we have the dist
> files committed with that in  mind.
> 
> - Definitely the impact in performance is huge. So we will go with your
> suggestion.
> 
> When do you need this changes, ASAP I guess?

It would be great to have Bower package as soon as possible so that we can deal with SWW updates easily.

Regarding performance - I think it's not really critical for Messages right now :)
Flags: needinfo?(azasypkin)
Just created the bower package, containing just the dist/sww.js file.
See Also: → bug 1173758
(Reporter)

Comment 5

2 years ago
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #4)
> Just created the bower package, containing just the dist/sww.js file.

Great, thanks!
(Reporter)

Updated

2 years ago
Depends on: 1174078
No longer depends on: 1173539
(Reporter)

Updated

2 years ago
Depends on: 1175045
(Reporter)

Updated

2 years ago
Depends on: 1175944
(Reporter)

Updated

2 years ago
See Also: → bug 1175949
(Reporter)

Comment 6

2 years ago
Rebased WIP on patch from bug 1183133, testing, trying to figure out why currently it looks even slower than without SW :)
(Reporter)

Comment 7

2 years ago
Hey Fernando,

I've tried to create very simple app that we can use to benchmark app with and without SW (packaged pre-installed, privileged app) [1]. This simple app consists of one HTML file, one js file that is internally makes one "fetch" request for the JSON payload. Every significant operation is instrumented by "permormance.mark" so that we can measure it with Raptor.

What we're trying to achieve is to have at least the same performance metrics as we have currently with app:// app.

At the moment it has 4 benchmarks:
* Baseline - simple app without any SW;
* Empty SW - the same app, but it registers empty SW file;
* SW with fetch only - the same app that registers SW that does "e.respondWith(fetch(e.request))" for every fetch request;
* SimpleOfflineCache - the same app with SimpleOfflineCache from ServiceWorkerWare.

At [2] you can see results from 20 Raptor runs. To see what every performance mark means you can look at app.js [3] or just ping me on IRC :)

Could you please help to understand/interpret what we see here? Is it expected that performance is dropped that significantly (see last p95 column)?

Also I see that "DOMContentLoaded" event handler is not fired when files are fetched by SW, is it expected as well?

Francisco you may be interested in the "SimpleOfflineCache" measurements as well :)

Thanks!

[1] https://github.com/azasypkin/sw-tests/tree/master/benchmark-app
[2] https://github.com/azasypkin/sw-tests#results
[3] https://github.com/azasypkin/sw-tests/blob/master/benchmark-app/empty-sw/js/app.js
Flags: needinfo?(francisco)
Flags: needinfo?(ferjmoreno)

Comment 8

2 years ago
Thanks for working on this Oleg!

(In reply to Oleg Zasypkin [:azasypkin] from comment #7)
> At [2] you can see results from 20 Raptor runs. To see what every
> performance mark means you can look at app.js [3] or just ping me on IRC :)
> 
> Could you please help to understand/interpret what we see here? Is it
> expected that performance is dropped that significantly (see last p95
> column)?

Well, we expect service workers to add some load time overhead, specially since no optimization has been done yet, but these numbers are certainly quite high.

I believe Francisco already mentioned to you a couple of issues with the tests that he is already trying to fix to get more accurate numbers.

> 
> Also I see that "DOMContentLoaded" event handler is not fired when files are
> fetched by SW, is it expected as well?
> 

No, this is not expected. Do you see the same with a service worker enabled hosted app? Could you file a bug for this, please?

Thanks!
Flags: needinfo?(ferjmoreno)
Hi all,

Thanks Oleg for this benchmarks, it's the perfect starting point for continue adding more and more measurements.

As Fernando commented we expect SW to add some initial latency, specially in the case of measuring small amount of files requested.

I ran your app and did some modification, and findings (all this code is on the forked repo: https://github.com/arcturus/sw-tests)

- First I modified all files to have a link to install the worker, and remove the worker registration from the app.js file. So now when you click on an app, then need to do another click on a link to register the worker. The idea behind this is testing (or trying) the same files for all the tests.

- Second, in the test related to the use of SWW and the SimpleOfflineCache middleware, I modified it to use another middleware that puts the content inside the offline cache, otherwise we were hitting the cache, failing and going to the network. The first interesting finding was the following, after doing this change, the numbers that I get are almost similar to the ones before the change:

Metric                               Mean     Median   Min      Max      StdDev  p95
-----------------------------------  -------  -------  -------  -------  ------  -------
coldlaunch.benchmark-js-file-parsed  682.750  671.000  644.000  743.000  32.561  742.000
coldlaunch.benchmark-load            685.250  673.000  647.000  745.000  32.789  744.500
coldlaunch.benchmark-payload-loaded  756.050  743.500  710.000  822.000  36.247  821.000
coldlaunch.benchmark-payload-read    758.000  745.500  717.000  823.000  35.996  822.000
coldlaunch.fullyLoaded               758.550  748.000  717.000  823.000  35.898  822.000
coldlaunch.pss                       16.130   16.100   16.000   16.200   0.071   16.200
coldlaunch.rss                       27.505   27.500   27.300   27.600   0.097   27.600
coldlaunch.uss                       11.570   11.600   11.400   11.600   0.071   11.600

Which means:
test 1: request -> sw interception -> cache match -> fail -> network fetch -> result
test 2: request -> sw interception -> cache match -> result

Means that the cache api is at least, without being optimized as fast as fetching a file via network request with the APP protocol. Not bad :)

- Another test that I performed is using SWW and doing a fetch through it, here is the result:

Metric                               Mean     Median   Min      Max      StdDev  p95
-----------------------------------  -------  -------  -------  -------  ------  -------
coldlaunch.benchmark-js-file-parsed  685.650  672.000  637.000  815.000  41.476  788.500
coldlaunch.benchmark-load            687.650  674.000  639.000  817.000  41.468  790.500
coldlaunch.benchmark-payload-loaded  754.250  743.000  706.000  895.000  43.063  860.500
coldlaunch.benchmark-payload-read    757.800  748.000  708.000  896.000  42.595  862.000
coldlaunch.fullyLoaded               758.450  748.500  708.000  897.000  42.603  863.000
coldlaunch.uss                       11.645   11.700   11.500   11.700   0.067   11.700
coldlaunch.pss                       16.180   16.200   16.000   16.200   0.060   16.200
coldlaunch.rss                       27.540   27.600   27.300   27.600   0.080   27.600

Those numbers are pretty similar to the ones that you got on your Simple Offline middleware that was performing the fetch (after failing on the cache), which again means the cache api is looking strong as numbers look pretty similar if we fail to find something in the cache.

- Another test that I performed was, trying to minimize the requests on the serviceworker, by instead of doing an importScripts of the sww library, including it directly on the sw.js file minimized, here are the results:

SWW minified in a single file
 
Metric                               Mean     Median   Min      Max      StdDev  p95
-----------------------------------  -------  -------  -------  -------  ------  -------
coldlaunch.benchmark-js-file-parsed  672.600  669.500  623.000  743.000  33.600  739.500
coldlaunch.benchmark-load            675.300  672.000  626.000  746.000  33.620  742.000
coldlaunch.benchmark-payload-loaded  742.400  740.500  696.000  841.000  36.361  819.500
coldlaunch.benchmark-payload-read    744.850  741.500  697.000  842.000  36.362  820.500
coldlaunch.fullyLoaded               745.400  741.500  697.000  843.000  36.149  821.000
coldlaunch.uss                       11.435   11.400   11.300   11.500   0.065   11.500
coldlaunch.pss                       15.990   16.000   15.800   16.100   0.070   16.100
coldlaunch.rss                       27.400   27.400   27.200   27.500   0.077   27.500 

Numbers are almost the same, just a tiny tiny tiny better, which again I think is good, seems the importing scripts from the SW is not an issue.

- Another point is SW is still not optimized to have a grace period for attending more request. What it means is that 1 sw was being launched to serve index.html and app.js, and after doing that was being killed, and another one has been launched to serve the request to payload.json. This is a platform thing that needs to be solved. Either waiting some seconds before killing a SW or implementing any other strategy. Wonder how much time we could save with that.

- Seems to me, tha the SWW library is adding around ~80ms per request when we compare the results without using it and when we start using it, which could be kind of ok, but I'm sure we can do better there.

- Definitely when we start using SW, no matter if it's using the library or using a simple SW the numbers get doubled, I heard Vivien talking about predeclaring the SW on the manifest so the system can be smart and try to prelaunch them, but again, we will need a lot of work in the platform here.

Thanks a lot for this first numbers, now from here, we need to get them better. There is one more test that I want to do, is doing the same tests, but with hosted apps, to compare them with app protocol just in case that something is different there.

Last thing, the numbers for the test I ran (yours + the extra ones that I added here):
https://pastebin.mozilla.org/8839691

Thanks again!
Flags: needinfo?(francisco)

Comment 10

2 years ago
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #9)
> - Another point is SW is still not optimized to have a grace period for
> attending more request. What it means is that 1 sw was being launched to
> serve index.html and app.js, and after doing that was being killed, and
> another one has been launched to serve the request to payload.json. This is
> a platform thing that needs to be solved. Either waiting some seconds before
> killing a SW or implementing any other strategy. Wonder how much time we
> could save with that.

This is definitely something we need to do.  Can you write a bug for it and have it block ServiceWorkers-v2?  I don't think it should be hard.  It just needs someone to work it.

And I'm glad to hear Cache is not showing up as a major bottleneck yet. :-)
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #8)
> > 
> > Also I see that "DOMContentLoaded" event handler is not fired when files are
> > fetched by SW, is it expected as well?
> > 
> 
> No, this is not expected. Do you see the same with a service worker enabled
> hosted app? Could you file a bug for this, please?

Sorry, my bad, I forgot that script listening for "DOMContentLoaded" is "async", so it seems that with ServiceWorker "DOMContentLoaded" is always fired before script starts to listen for this event and that sounds pretty normal to me :) With "defer" script DOMContentLoaded is always fired. 

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #9)
> I ran your app and did some modification, and findings (all this code is on
> the forked repo: https://github.com/arcturus/sw-tests)

Great findings! Could you please send PR with your changes?

> - Second, in the test related to the use of SWW and the SimpleOfflineCache
> middleware, I modified it to use another middleware that puts the content
> inside the offline cache, otherwise we were hitting the cache, failing and
> going to the network. 

Mmm, should not "request -> sw interception -> cache match -> fail -> network fetch" flow be only for the first run and don't really affect numbers for 20 RUNS? I thought that if SimpleOfflineCache can't find resource in the cache, it requests the network, _caches result_ and then respond so that for the consequent requests resource will be taken from cache?

Thanks!
Hi Oleg,

just send you a PR.

Regarding your last question, we just changed the behavior of the SimpleOfflineCache to not to cache by default. Just to do the fetch, since that's an option that a developer should add if wanted.
(Reporter)

Updated

2 years ago
Depends on: 1186949
Mass closing of Gaia::SMS bugs. End of an era :(
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → WONTFIX
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in before you can comment on or make changes to this bug.