1197019 - Update Camera to be ready for L10n API v3

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect)

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: pull request

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8650791 [details] [review]
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 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8650791 [details] [review]
pull request

Trying a different reviewer.

Comment 4

[David Flanagan [:djf]]

Comment on attachment 8650791 [details] [review]
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 5

[Wilson Page [:wilsonpage]]

Comment on attachment 8650791 [details] [review]
pull request

Some nit/questions inline.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8650791 [details] [review]
pull request

updated the patch, rerequesting review.

Comment 7

[Wilson Page [:wilsonpage]]

Comment on attachment 8650791 [details] [review]
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?

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

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 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8650791 [details] [review]
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   |

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

Thanks!

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

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.