834620 - Begin initializing camera hardware while initializing dom / css / js

Status: VERIFIED FIXED

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

Description

[James Lal [:lightsofapollo]]

I don't have hard data on this aside from my dump statements but it looks like we can shave off about 300-900ms on the time it takes to generate the first preview by initializing the hardware while parsing the dom ( by using a seperate js file not deferred before the css )...

See my hack: https://github.com/lightsofapollo/gaia/compare/start-camera-init-really-early

Comment 1

[David Scravaglieri [:scravag]]

Dale, can you investigate on this and give us feedback to decide for blocking or not ?

Comment 2

[Dale Harvey (:daleharvey)]

I think this is a good idea, the hardware abstraction was planned anyway since the camera API became somewhat unweildy and needs to be seperated.

And my testing is similiar to James's, I will still need to test with everything fully working but it looks like around a 400ms speedup (and an extra 3/400ms with the async storage change)

Which are both significant speedups with not very intrusive changes, I will have both patches done tonight and suggest this blocks

Cheers James for investigating.

Comment 3

[Andrew Overholt [:overholt]]

Okay, we'll block on this.  Please give us a heads up if your fix(es) end up with the potential for serious regressions.  Thanks.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Marking status-b2g18 and status-b2g18-v1.0.0 as affected, please update the status to fixed once this is verified landed on v1-train/mozilla-b2g18 and v1.0.0/mozilla-b2g18_v_1_0_0

Comment 5

[David Scravaglieri [:scravag]]

Dale, any update ?

Comment 6

[Dale Harvey (:daleharvey)]

I am going to do more experimenting, but I dont believe there are significant gains in this approach anymore. I was seeing times around 150ms for parsing assets, and initialising the hardware early doesnt necessarily means it will be ready earlier. 

https://bugzilla.mozilla.org/show_bug.cgi?id=821695 is much more important, working on that now

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

No longer blocking based on comment 6 and the doubt that we'll get huge wins here. Re-nom if that changes.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: Bugs marked status-b2g18: affected after 2/13 branching of v1.0.1 are now also status-b2g18-v1.0.1: affected

Comment 9

[David Scravaglieri [:scravag]]

Dale will provide a patch in few hours.

Comment 10

[Dale Harvey (:daleharvey)]

Attachment: Github PR

Comment 11

[Dale Harvey (:daleharvey)]

Attachment: Github PR

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 721895 [details]
Github PR

https://github.com/mozilla-b2g/gaia/commit/1f7dbc2534914620ba05126e99eb47991de758e6

Comment 13

[Andrew Overholt [:overholt]]

tef? if we want this on 1.0.1 for bug 817051.

Comment 14

[Michael Vines [:m1] [:evilmachines]]

(John, can we get a v1.0.1 uplift please?)

Comment 15

[Julien Wajsberg [:julienw]]

This needs bug 841251. Please don't uplift until Bug 841251 is uplifted.

Comment 16

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #14)
> (John, can we get a v1.0.1 uplift please?)

Still waiting on 841251 for v1.0.1

v1-train: 0542a2f3c2f33274a90082011a13ef5d8a8c315f

Comment 17

[Julien Wajsberg [:julienw]]

John, sorry for the mess.. Bug 841251 is not in v1-train yet :(

Comment 18

[John Ford [:jhford] CET/CEST Berlin Time]

Backed out as 42fb0a150b45ede69a51dd73af3a04ea1c7717b0

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit 1f7dbc2534914620ba05126e99eb47991de758e6 as:
v1-train: d0eeb90cddbdedd3c521d42f86a97ef33941d487
v1.0.1: 947a6cb04f500acaabb36d252c6464ea6475556d

Comment 20

[amiller]

Does not make sense to create a regression issue.

Comment 21

[amiller]

Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 22

[Dale Harvey (:daleharvey)]

There was no functional change so I dont understand what steps to verify would be except that the app is as functional as it previously was

Comment 23

[amiller]

Verified fixed per comment 22.

Comment 24

[John Hammink]

Probably best tested with datazilla.