system app is currently blacklisted from JS aggregation due to a bug that has been resolved in April. https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-optimize.js#L38 Aggregating JS seems to shave 1mb from memory usage of the system app and when compared two phones next to each other it seems to shave ~1s from startup time (Keon, master).

Comment on attachment 8336478 [details] [diff] [review] bug941969.diff Review of attachment 8336478 [details] [diff] [review]: ----------------------------------------------------------------- It hasn't been fixed by bug 839574. That bug is only about blacklisting apps. But anyway, it seems to work fine now on desktop, device and firefox.

Do we have something blocking this patch to be merged?

PR, carrying over r+. I'll wait for tests and land it

I tested it on Flame, master, using make test-perf with 10 runs. It shaved 3mb of ram. How far back can we backport it?

(In reply to Zibi Braniecki [:gandalf] from comment #5) > I tested it on Flame, master, using make test-perf with 10 runs. It shaved > 3mb of ram. > > How far back can we backport it? I recommend 2.0/2.1/2.2, but that's subject to release manager approval.

(In reply to Zibi Braniecki [:gandalf] from comment #5) > I tested it on Flame, master, using make test-perf with 10 runs. It shaved > 3mb of ram. > > How far back can we backport it? How did you measure that?

(In reply to Fabrice Desré [:fabrice] from comment #7) > (In reply to Zibi Braniecki [:gandalf] from comment #5) > > I tested it on Flame, master, using make test-perf with 10 runs. It shaved > > 3mb of ram. > > > > How far back can we backport it? > > How did you measure that? I used make test-perf on Settings app, and it shows memory for Settings and System app. Does those result have any value? I also tested bootstrap time using a stopwatch, and on Flame it shaves a little bit under 1s (5 runs, very consistently). I remember that on Keon it was above 1s (see comment 0)

Comment on attachment 8504040 [details] pull request [Approval Request Comment] [Bug caused by] (feature/regressing bug #): None [User impact] if declined: Performance/Memory cost [Testing completed]: None [Risk to taking this patch] (and alternatives if risky): Low, because any regressions will be very visible (all System JS is minified) and reverting is a one liner [String changes made]: None

Landed on master: Commit: https://github.com/mozilla-b2g/gaia/commit/66e26679d4ecd7b1d0a013555e3b33871354046d Merge: https://github.com/mozilla-b2g/gaia/commit/de254419f3553f48187d003ee8e38034b429f069

I also tested performance impact using the latest system test-perf (bug 1070615). (SAMPLE=8) startup > mozSystemLoadEnd - 2857 -> 2538 startup > mozOsLogoEnd - 15551 -> 15255 startup > mozHomescreenStart - 14343 -> 13986 All results are statistically significant (p-value < 0.05) So, it seems that on Flame we have ~300ms improvement. I expect a more significant one on lower-end phones.

(In reply to Zibi Braniecki [:gandalf] from comment #8) > (In reply to Fabrice Desré [:fabrice] from comment #7) > > (In reply to Zibi Braniecki [:gandalf] from comment #5) > > > I tested it on Flame, master, using make test-perf with 10 runs. It shaved > > > 3mb of ram. > > > > > > How far back can we backport it? > > > > How did you measure that? > > I used make test-perf on Settings app, and it shows memory for Settings and > System app. Does those result have any value? > > I also tested bootstrap time using a stopwatch, and on Flame it shaves a > little bit under 1s (5 runs, very consistently). > > I remember that on Keon it was above 1s (see comment 0) I don't see improvements in datazilla for now: https://datazilla.mozilla.org/b2g/?branch=master&device=flame-319MB&range=7&test=settings_memory&app_list=calendar,camera,clock,communications/contacts,communications/dialer,costcontrol,email%20FTU,fm,gallery,music,settings,sms,system_rss,video&app=settings&gaia_rev=1a85a538cb0a860b&gecko_rev=7090d5b4ed1e&plot=avg

(In reply to Fabrice Desré [:fabrice] from comment #12) > (In reply to Zibi Braniecki [:gandalf] from comment #8) > > (In reply to Fabrice Desré [:fabrice] from comment #7) > > > (In reply to Zibi Braniecki [:gandalf] from comment #5) > > > > I tested it on Flame, master, using make test-perf with 10 runs. It shaved > > > > 3mb of ram. > > > > > > > > How far back can we backport it? > > > > > > How did you measure that? > > > > I used make test-perf on Settings app, and it shows memory for Settings and > > System app. Does those result have any value? > > > > I also tested bootstrap time using a stopwatch, and on Flame it shaves a > > little bit under 1s (5 runs, very consistently). > > > > I remember that on Keon it was above 1s (see comment 0) > > I don't see improvements in datazilla for now: > https://datazilla.mozilla.org/b2g/?branch=master&device=flame- > 319MB&range=7&test=settings_memory&app_list=calendar,camera,clock, > communications/contacts,communications/dialer,costcontrol,email%20FTU,fm, > gallery,music,settings,sms,system_rss, > video&app=settings&gaia_rev=1a85a538cb0a860b&gecko_rev=7090d5b4ed1e&plot=avg :gandalf, any comments here? Unless there is proven improvements, this is better riding the trains and getting fixed in 2.2 than being uplifted. Not sure if there is a reason on missing the improvements in datazilla, so please let us know...

This patch is breaking b2g desktop production builds, as well as flatfish ones. We need to backout.

If scripts are merged into one, then we should try to fix original error from each script, or it may probably break entire app. I tried to do below items with your patch and it works fine with GAIA_OPTIMIZE=1 on b2g-desktop, 1. Change loading order of bootstrap.js and its requiring script , bootstrap.js should put on the bottom. 2. fix error for call_forwarding.js/icc.js/nfc_handover_manager.js which may have error on device without navigator.mozIccManager , mozMobileConnections ... etc.

I had a work week and didn't manage to look into this. It's a one-liner so backing out should be trivial. I can back it out or we can fix the real bug as :gduan suggests. Let me know and I'll do this first thing in the morning. Keeping NI to verify the memory/perf wins.

(In reply to Fabrice Desré [:fabrice] from comment #14) > This patch is breaking b2g desktop production builds, as well as flatfish > ones. We need to backout. That is also breaking Mulet, as documented in bug 1082001 comment 25.

(In reply to Zibi Braniecki [:gandalf] from comment #4) > Created attachment 8504040 [details] > pull request > > PR, carrying over r+. I'll wait for tests and land it Do we know why this has not been caught by Gaia Try? Are "B2G Desktop Opt" not the same than "B2G Desktop Production Builds"?

I guess tbpl doesn't run marionette with GAIA_OPTIMIZE=1 or production. 10:25:11 INFO - Running command: ['make', 'test-integration', 'NPM_REGISTRY=http://npm-mirror.pub.build.mozilla.org', 'REPORTER=mocha-tbpl-reporter', 'TEST_MANIFEST=./shared/test/integration/tbpl-manifest.json'] in /builds/slave/test/gaia

Backed out for comment 14, and being asked to do so over IRC. I guess we should implement the suggestions in comment 15. We should also see if we can run marionette tests with a GAIA_OPTIMIZE=1 profile. https://github.com/mozilla-b2g/gaia/commit/27a1d1baaa8e375b70e043efee67d5f2206c330b

I'd say even PRODUCTION=1, for all marionette tests.

Now that this patch is backed out, can we file follow up bugs?

Comment on attachment 8504040 [details] pull request Clearing approvals until it sticks in master.

Steal it for bug 1094339.

(In reply to Julien Wajsberg [:julienw] from comment #21) > I'd say even PRODUCTION=1, for all marionette tests. We can't do PRODUCTION=1. Some apps needed for tests would not be built in with that config. Unless we remote these test apps and have them dynamically installed in the marionette tests itself.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #26) > We can't do PRODUCTION=1. Some apps needed for tests would not be built in > with that config. > Unless we remote these test apps and have them dynamically installed in the > marionette tests itself. I would personally prefer to "install" these along with the apps property when we create the client. This should allow for smaller profile sizes and faster flash times for developers. For example: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/test/marionette/software_home_callscreen_test.js#L20 This should definitely be a follow-up though.

Comment on attachment 8519690 [details] PR to master Hi Alive, this patch works fine on device and b2g-desktop with GAIA_OPTIMIZE=1, please kindly check.

master: https://github.com/mozilla-b2g/gaia/commit/e632da1eebc3edefdd8947e977f1d386c0d4c794