Once the serviceworkerware[1] is ready, we will need to utilize it in our codebase and update the script for updating all the external lib including threads and this one. [1] https://github.com/gaia-components/serviceworkerware

Since using bower install will wrap the source into dist/sww.js because bower.json already defined it, do we only need to copy the packaged sww.js to lib folder after bower install in the script, or you would prefer to keep the original source in the bower component?

(In reply to Steve Chung [:steveck] from comment #1) > Since using bower install will wrap the source into dist/sww.js because > bower.json already defined it, do we only need to copy the packaged sww.js > to lib folder after bower install in the script, or you would prefer to keep > the original source in the bower component? As for now, I'm fine with "dist/sww.js" only.

Both work for me, but I suggest to keep consistency and copy it in lib/

I think my previous comment was a bit ambiguous, so let me clarify what I meant :) I think we need: "bower_components/serviceworkerware/dist/sww.js" ---> "lib/sww.js" (no need in sww.js.map for now); not: "bower_components/serviceworkerware/dist/sww.js" ---> "dist/sww.js" :)

Hey, I added the serviceworkerware into and update the threads, please tell me if you have any thought about updating the libs, thanks!

Comment on attachment 8624021 [details] Link to github Looks good! The only nit left at GitHub, i.e. we need more generic name for our shell script now. Thanks!

(In reply to Oleg Zasypkin [:azasypkin] from comment #6) > Comment on attachment 8624021 [details] > Link to github > > Looks good! The only nit left at GitHub, i.e. we need more generic name for > our shell script now. > Thanks for the reminder, I update the name to 'update_nga_libs.sh'

Hi :steveck I am pretty confident that this patch introduces an app start-up performance regression in the SMS app. I noticed it via the Raptor performance tests (not posting to treeherder, but still running). I retriggered the raptor launch test suite several times on your PR, and each time a significant SMS app launch time regression was detected: http://docs.taskcluster.net/tools/task-inspector/#4BjvmHYtRuuR3DcFhZuPug/0 http://docs.taskcluster.net/tools/task-inspector/#9ksEdtyVQe-QqL40L8PlOg/2 http://docs.taskcluster.net/tools/task-inspector/#TAeau0nmT0WjBq8GPMsNLw/2 https://tools.taskcluster.net/task-inspector/#I6W3XFPrR0umHCcAwPMNlg/0 Flagging as :needinfo so you see these results, thanks

I think this comes from the bridge change in https://github.com/mozilla-b2g/gaia/pull/30642/files#diff-89cb758a8e79fdb633ccb78053c9dd3eL1732 Calling setReady is calling serviceReady which is sending an event which in turns do more stuff. Steve maybe you can remove the change to the bridge from this PR for now? NI Wilson so that he's aware of the issue.

(In reply to Julien Wajsberg [:julienw] from comment #9) > I think this comes from the bridge change in > > https://github.com/mozilla-b2g/gaia/pull/30642/files#diff- > 89cb758a8e79fdb633ccb78053c9dd3eL1732 > > Calling setReady is calling serviceReady which is sending an event which in > turns do more stuff. > > Steve maybe you can remove the change to the bridge from this PR for now? > > NI Wilson so that he's aware of the issue. Interesting. I thought that removing the setTimeout, would speed up connection times as we're no longer deferring any handshake logic. IIRC Oleg wanted it removing too for some other reason.

Well, all this sounds very strange to me, when Messages app is run from HomeScreen (I think it's the case for raptor tests), we don't create any bridge service or client yet, so bridge should not affect anything here unless it does something on the JS file parsing stage. I'll try to dig into it today and see what is really happening.

Here is what I have locally using the following commands (for each revision): 1. "make raptor"; 2. "RUNS=10 APP=sms node tests/raptor/launch_test". With the latest PR (cc3c5537a5876ccd2eac3bd0dc1edc774d5507d0) Metric Mean Median Min Max StdDev p95 -------------------------------- -------- -------- -------- -------- ------ -------- coldlaunch.navigationLoaded 881.400 900.000 830.000 915.000 33.664 915.000 coldlaunch.willRenderThreads 915.100 931.000 862.000 949.000 32.709 949.000 coldlaunch.navigationInteractive 918.000 933.500 864.000 952.000 32.066 952.000 coldlaunch.visuallyLoaded 1039.600 1050.500 994.000 1071.000 28.200 1071.000 coldlaunch.fullyLoaded 1164.700 1168.000 1115.000 1209.000 29.692 1209.000 coldlaunch.uss 14.780 14.800 14.300 15.100 0.244 15.100 coldlaunch.rss 33.800 33.900 32.700 34.100 0.405 34.100 coldlaunch.pss 19.260 19.300 18.500 19.600 0.320 19.600 coldlaunch.contentInteractive 1493.300 1495.000 1446.000 1545.000 30.695 1545.000 coldlaunch.objectsInitEnd 1536.167 1544.500 1499.000 1563.000 26.630 n/a With the PR's HEAD~1 (2b75846f9b35620ddf8ffadc66ef2d2a650b65db), aka base revision Metric Mean Median Min Max StdDev p95 -------------------------------- -------- -------- -------- -------- ------ -------- coldlaunch.navigationLoaded 888.000 884.500 830.000 972.000 38.486 972.000 coldlaunch.willRenderThreads 923.900 921.500 863.000 1009.000 39.175 1009.000 coldlaunch.navigationInteractive 926.600 924.000 865.000 1013.000 39.543 1013.000 coldlaunch.visuallyLoaded 1049.400 1044.500 996.000 1136.000 38.352 1136.000 coldlaunch.fullyLoaded 1178.200 1168.500 1130.000 1249.000 34.333 1249.000 coldlaunch.uss 14.720 14.750 14.400 15.000 0.218 15.000 coldlaunch.pss 19.190 19.100 19.000 19.500 0.207 19.500 coldlaunch.rss 33.720 33.700 33.300 34.100 0.244 34.100 coldlaunch.contentInteractive 1505.800 1500.000 1448.000 1566.000 31.818 1566.000 coldlaunch.objectsInitEnd 1550.625 1547.000 1490.000 1617.000 36.602 n/a I don't see almost any difference between raptor results. Hey Robert, am I doing something wrong or interpret results incorrectly?

Hi Oleg, We are using the p95 value of the coldlaunch.visuallyLoaded metric, for 30 RUNS. There is a difference: Base rev: 1136.000 ms Patch rev: 1071.000 ms Diff: 65 ms or 5.7% faster launch time The values are lower on device, on gaia-ci / treeherder the raptor tests run on emulator which is alot slower and has greater variance. I would try running with RUNS=30, and do that a few times and you should see the consistent improvement.

(In reply to Robert Wood [:rwood] from comment #13) > Hi Oleg, > > We are using the p95 value of the coldlaunch.visuallyLoaded metric, for 30 > RUNS. There is a difference: > > Base rev: 1136.000 ms > Patch rev: 1071.000 ms > Diff: 65 ms or 5.7% faster launch time Mmm, that means that with the patch it's even faster, right? :) > > The values are lower on device, on gaia-ci / treeherder the raptor tests run > on emulator which is alot slower and has greater variance. > > I would try running with RUNS=30, and do that a few times and you should see > the consistent improvement. So I tried 3 times with RUNS=30, without data and with light workload. Here are results (see attached file for full results) for p95, coldlaunch.visuallyLoaded metric: Latest PR (cc3c5537a5876ccd2eac3bd0dc1edc774d5507d0), without any data: [1086.000, 1069.000, 1070.000] PR's HEAD~1 (2b75846f9b35620ddf8ffadc66ef2d2a650b65db), aka base revision, no data [1058.000, 1059.000, 1123.000] Latest PR (cc3c5537a5876ccd2eac3bd0dc1edc774d5507d0), with light workload [1308.000, 1337.000, 1372.000] PR's HEAD~1 (2b75846f9b35620ddf8ffadc66ef2d2a650b65db), aka base revision, with light workload [1363.000, 1301.000, 1295.000] So the max difference I can see here is about 6% that looks like margin of error rather than regression... Maybe we need to trigger tests on Treeherder again and see if something has changed.

To get the most accurate results you should actually merge all your runs in one run and do the calculation. Having 3 different runs does not give more confidence :) That said, I agree there does not look to have an issue here. I think this was a false positive. Moreover we _know_ that working on NGA will give performance regression and that we'll work on this later on. So let's move forward. Thanks Robert for the heads up though, please report again if you see something in the future, that's really appreciated !

(In reply to Julien Wajsberg [:julienw] from comment #9) > I think this comes from the bridge change in > > https://github.com/mozilla-b2g/gaia/pull/30642/files#diff- > 89cb758a8e79fdb633ccb78053c9dd3eL1732 > > Calling setReady is calling serviceReady which is sending an event which in > turns do more stuff. > > Steve maybe you can remove the change to the bridge from this PR for now? > Anyway I still split the patch to simply including the sww lib in this patch, and we can create another bug for tracking the possible regression for the latest threads lib.

Comment on attachment 8624021 [details] Link to github r=me let's move forward here !

(In reply to Julien Wajsberg [:julienw] from comment #15) > To get the most accurate results you should actually merge all your runs in > one run and do the calculation. > > Having 3 different runs does not give more confidence :) Well it should have made you more confident! I separated it intentionally to do the same thing as Treeherder does when raptor job is re-run 3 times: it runs tests 30 times for PR revision, then 30 times for base revision, compares the difference and fails the job if performance regression exceeds "acceptable threshold". So none of those 3 local runs would trigger alarm :)

Let's land the sww first and track the regression later. In master: https://github.com/mozilla-b2g/gaia/commit/7d2e76e37919f85f4883efbcccf503ffae2e02c1