Comment on attachment 8650791 [details] pull request Andrew, can you review this patch for me please? Context: We're working on a next-generation l10n library that, among other goodies, is fully asynchronous. To prepare for that, we're moving away from all synchronous mozL10n.get calls. On top of that we're migrating our date/time formatting to use Intl API - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl

Comment on attachment 8650791 [details] pull request Trying a different reviewer.

Comment on attachment 8650791 [details] pull request I'm not the right reviewer for this, either. I think you need Wilson or Justin for this one. Wilson, Justin: can one of you take this review, please?

Comment on attachment 8650791 [details] pull request Some nit/questions inline.

Comment on attachment 8650791 [details] pull request updated the patch, rerequesting review.

Comment on attachment 8650791 [details] pull request Just like to see some performance numbers from Flame to clarify that this approach isn't regressing startup. BTW how much is l10n/l20.js expected to add to an app's startup time?

Yeah! I didn't forget - I just updated to node 4 and now raptor doesn't work ;) (bug 1202921). wrt. expectations - so far we see 0 to -100ms performance difference between l10n.js/l20n.js and 0 to +100kb of memory.

Comment on attachment 8650791 [details] pull request Ok, I learned how to use nvm to manage my node versions. Flame-kk from today with 31 runs master: | Metric | Mean | Median | Min | Max | StdDev | p95 | | --------------------- | -------- | ------ | ------ | ------ | ------- | -------- | | navigationLoaded | 4096.226 | 4104 | 3420 | 4249 | 134.839 | 4222.900 | | navigationInteractive | 4096.613 | 4104 | 3420 | 4249 | 134.866 | 4222.900 | | visuallyLoaded | 4327.581 | 4329 | 3663 | 4476 | 134.869 | 4476 | | contentInteractive | 5245.548 | 5247 | 4558 | 5390 | 137.387 | 5384.450 | | fullyLoaded | 5245.903 | 5248 | 4558 | 5391 | 137.490 | 5384.450 | | pss | 24.454 | 24.451 | 24.372 | 24.604 | 0.043 | 24.523 | | uss | 19.469 | 19.465 | 19.387 | 19.609 | 0.041 | 19.538 | | rss | 41.852 | 41.852 | 41.770 | 42.016 | 0.045 | 41.921 | patch: | Metric | Mean | Median | Min | Max | StdDev | p95 | | --------------------- | -------- | ------ | ------ | ------ | ------- | -------- | | navigationLoaded | 3996.871 | 4011 | 3456 | 4153 | 115.768 | 4094 | | navigationInteractive | 3997.290 | 4011 | 3457 | 4154 | 115.684 | 4095 | | visuallyLoaded | 4357 | 4370 | 3807 | 4492 | 119.202 | 4473.950 | | contentInteractive | 5173.968 | 5202 | 4624 | 5332 | 118.466 | 5308.550 | | fullyLoaded | 5174.452 | 5202 | 4625 | 5332 | 118.480 | 5308.600 | | uss | 19.521 | 19.543 | 19.402 | 19.660 | 0.067 | 19.617 | | rss | 41.890 | 41.922 | 41.758 | 42.039 | 0.075 | 41.992 | | pss | 24.504 | 24.529 | 24.380 | 24.647 | 0.070 | 24.603 |

Commit: https://github.com/mozilla-b2g/gaia/commit/eb9d29d5af8ff5b2345b77e1ab30ab4ad4987fe9 Thanks!

Nice wins visible on Aries and Flame in performance, but there seem to be a regression in memory. I'll investigate on Monday.

I can't reproduce any memory regression for this patch. The raptor numbers looked at from the perspective of three days look better. On Flame, the USS before landing (25th 10:10am mark) was around 18.9mb. The mark right after landing (13:13) jumped to 20.4 which is worrying, but after that the numbers drop over the weekend to 18.4-18.1mb. That makes me feel that the first result was likely a fluke. I also tested Z3 locally with and without the patch again and got the same results as in comment 9 - delta within 60kb, stdev within 60kb. Wrt. performance, it looks better. On Flame, the navigationLoaded seems to drop from ~5.4s to 5.2s while fullyLoaded from 7.7s, to 7.5s with similar win on Z3 where fullyLoaded dropped from 3.2 to 3.0s so <200ms win. If you see any regressions that I didn't detect, please, let me know.