Closed
Bug 994077
Opened 11 years ago
Closed 11 years ago
[Camera][Hamachi][Dolphin] Long wait for initial preview stream
Categories
(Firefox OS Graveyard :: Gaia::Camera, defect, P1)
Tracking
(blocking-b2g:2.0+, b2g-v1.4 affected, b2g-v2.0 fixed)
People
(Reporter: wilsonpage, Assigned: wilsonpage)
References
Details
(Keywords: perf, Whiteboard: [c=progress p= s=2014.04.25 u=2.0][sprd315883])
Attachments
(2 files, 1 obsolete file)
I've noticed that Hamachi can sometimes take > 6sec to retrieve the camera preview stream. Wondering if there is anything that can be done on Gecko side to speed this up.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #8403984 -
Flags: feedback?(mhabicher)
Assignee | ||
Updated•11 years ago
|
blocking-b2g: --- → 1.4?
Comment 2•11 years ago
|
||
The last time I measured this, ~2 seconds of the start-up time can be attributed to starting the driver and opening the camera hardware, with about another 100ms of overhead across everything else in Gecko.
See bug 817051, bug 821841, bug 833114, and bug 835367.
Can you take some measurements? Specifically, how long it takes getCamera() to return, and the time between getCamera() returning and its onSuccess callback getting invoked.
Assignee | ||
Comment 3•11 years ago
|
||
On my Hamachi I record 3 seconds between request and callback. If this is unavoidable, we can just close this issue now.
Comment 4•11 years ago
|
||
(In reply to Wilson Page [:wilsonpage] from comment #3)
>
> On my Hamachi I record 3 seconds between request and callback. If this is
> unavoidable, we can just close this issue now.
There are a lot of historical bugs related to Camera start-up time. Take a look at bug 914916 comment 4 for some detailed information, and also bug 915083.
Regardless, taking ~6 seconds to launch is far too long.
Comment 5•11 years ago
|
||
6 secs!...not acceptable, lets investigate what is going on
Severity: normal → blocker
blocking-b2g: 1.4? → 1.4+
Flags: needinfo?(wilsonpage)
Updated•11 years ago
|
Hardware: x86 → ARM
Assignee | ||
Comment 6•11 years ago
|
||
I'm currently working on trimming JS startup time to an absolute minimum to improve the 'critical-path' (time to viewfinder). On the Hamachi the majority of this time is waiting for Gecko to give us the camera object.
Flags: needinfo?(wilsonpage)
Comment 7•11 years ago
|
||
I am leaving this assigned to you since you are currently profiling and figure out whats going on?
Assignee: nobody → wilsonpage
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8406224 -
Flags: review?(dmarcos)
Assignee | ||
Comment 10•11 years ago
|
||
This is a big patch. Probably risky to uplift this to 1.4.
Comment 12•11 years ago
|
||
You can land after addressing comments on github.
Updated•11 years ago
|
Attachment #8406224 -
Flags: review?(dmarcos) → review+
Comment 13•11 years ago
|
||
It's important tho check how these changes affect the numbers in perf-o-matic:
https://datazilla.mozilla.org/b2g/?branch=master&device=hamachi&range=7&test=cold_load_time&app_list=camera&app=camera&gaia_rev=553da99ab09b6b89&gecko_rev=5b6e82e7bbbf&plot=avg
Comment 14•11 years ago
|
||
(In reply to Wilson Page [:wilsonpage] from comment #10)
> This is a big patch. Probably risky to uplift this to 1.4.
Hema, based on what Wilson is saying, do we really think this should still be a 1.4+ blocker? I do agree that the launch time is noticably slow (tested myself), but i want to assess the risk of landing here. Please comment, thanks.
Updated•11 years ago
|
Priority: -- → P1
Whiteboard: [c= p= s= u=]
Comment 15•11 years ago
|
||
As discussed earlier, this patch is big and risky to land into 1.4. Can you check to see if there is a low-risk change from your work that will help us on the perf front that we can comfortably take in for 1.4 release?
Thanks for this work including the tests that you have added/updated in this patch.
Once the changes are reviewed (also get djf to do one more review since it is a big patch - 2 pairs of eyes will be good) with necessary tests, we can land them into master for 2.0 release. It will give us some time to bake and fix any issues for next release.
Back into 1.4? bucket for reconsideration after I hear back from you on low-risk fixes if any for 1.4
Thanks
Hema
Thanks
Hema
blocking-b2g: 1.4+ → 1.4?
Comment 16•11 years ago
|
||
The one problem I think we have here is the fact that the viewfinder is taking too long to load could be a blocker against our performance certification criteria.
Question:
Is this issue device independent or specific to Buri?
Flags: needinfo?(hkoka)
Comment 18•11 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #17)
> What's the performance criteria?
Mike might know that answer.
Flags: needinfo?(jsmith) → needinfo?(mlee)
I don't think we have a hard internal acceptance number defined for this yet. Our numbers are going to be pretty coarse-grained for 1.4 given timeframe and probably won't include many app-internal operations.
That said, if this is considered to be part of camera cold-launch, our rough guideline for *any* cold launch to user-ready is ~1.25s max.
Camera's one of them that I know we'd have to be looser on (I don't think Android's even near that target) but 6s is way, way too high IMO. For comparison, we already have bug 995303 against 1.3T/Tarako at 2.9 secs. I'd expect 1.4/Hamachi to be faster.
But all said, Jason's referring to external criteria via IOT, partner, etc. I haven't heard much about camera performance stats there, but I expect this to get called out if we leave it as-is.
Comment 20•11 years ago
|
||
Do you have a consistent way to reproduce those 6s on startup time that you claim? I'm collecting numbers but I haven't been able to see such a long startup time on my hamachi.
Flags: needinfo?(wilsonpage)
Comment 21•11 years ago
|
||
Geo is correct, the start-up time goal for all apps is 1.25 seconds [1]. If that is not possible, then some UI needs to be shown that gives the user indication that something is happening.
[1] https://developer.mozilla.org/en-US/Apps/Build/Performance/Firefox_OS_app_responsiveness_guidelines
Flags: needinfo?(mlee)
Comment 22•11 years ago
|
||
(In reply to Eli Perelman from comment #21)
>
> Geo is correct, the start-up time goal for all apps is 1.25 seconds [1]. If
> that is not possible, then some UI needs to be shown that gives the user
> indication that something is happening.
Then we'll need something like that for Hamachi and Buri, as the camera hardware on these devices takes ~2s to complete the single call to connect().
Updated•11 years ago
|
blocking-b2g: 1.4? → 2.0?
Comment 23•11 years ago
|
||
I think this needs more discussion before we conclude moving this to a future release.
blocking-b2g: 2.0? → 1.4?
Comment 24•11 years ago
|
||
Discussed this with rel man - here's what we should do for 1.4:
For 1.4, we will not take the same patch we are taking for master. However, we'll need to come up with something more low risk to mitigate the fact that there's a long startup time for the viewfinder on devices like Buri. The suggestion that comment 21 seems like the right direction - let's implementation a progress indicator when the viewfinder hasn't loaded. To simplify tracking, I'm going to open a new bug for this & move this bug out to 2.0.
blocking-b2g: 1.4? → 2.0?
Comment 25•11 years ago
|
||
bug 999171 has been filed to track the 1.4 solution.
Updated•11 years ago
|
Flags: needinfo?(hkoka)
Comment 26•11 years ago
|
||
I've been playing today with my Hamachi and I cannot reproduce the 6s that wilson claims. The time from tap to first picture is around 3.5s (Around 2.5s waiting for the camera hardware to provide the preview). This startup time has remained unchanged since 1.2. We have to try to improve this but the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I don't know who has the expertise to work on this. Mike might point us to the right direction
Flags: needinfo?(mhabicher)
Comment 27•11 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #26)
> I've been playing today with my Hamachi and I cannot reproduce the 6s that
> wilson claims. The time from tap to first picture is around 3.5s (Around
> 2.5s waiting for the camera hardware to provide the preview). This startup
> time has remained unchanged since 1.2. We have to try to improve this but
> the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I
> don't know who has the expertise to work on this. Mike might point us to the
> right direction
This doesn't always happen 100% of the time. Of the three times I tried, I reproduced this 2/3 times.
Comment 28•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #27)
> (In reply to Diego Marcos [:dmarcos] from comment #26)
> > I've been playing today with my Hamachi and I cannot reproduce the 6s that
> > wilson claims. The time from tap to first picture is around 3.5s (Around
> > 2.5s waiting for the camera hardware to provide the preview). This startup
> > time has remained unchanged since 1.2. We have to try to improve this but
> > the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I
> > don't know who has the expertise to work on this. Mike might point us to the
> > right direction
>
> This doesn't always happen 100% of the time. Of the three times I tried, I
> reproduced this 2/3 times.
I tested this btw by doing a cold launch of the camera app with 1.3 & 1.4 next to each other.
Comment 29•11 years ago
|
||
To address the long waits for the camera preview on hamachi we need a loading bar / icon to keep the user informed about what's going on.
Flags: needinfo?(tshakespeare)
Flags: needinfo?(amlee)
Comment 30•11 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #29)
> To address the long waits for the camera preview on hamachi we need a
> loading bar / icon to keep the user informed about what's going on.
Note - I already needinfo UX over in bug 999171 about this.
Assignee | ||
Comment 31•11 years ago
|
||
dmarcos: I guess the 6sec startup time could be influenced by many platform variables. Justin said he noticed improvements over time. This makes this especially hard to work on as we can't determine where improvements and regressions are coming from.
If we really want the gains this patch brings in 1.4, perhaps it would be more efficient to polish and test the patch. It does have test coverage in areas that master doesn't currently have. I'd be comfortable landing if we got more eyes on this.
Flags: needinfo?(wilsonpage)
Updated•11 years ago
|
Comment 32•11 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #26)
>
> This startup
> time has remained unchanged since 1.2. We have to try to improve this but
> the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I
> don't know who has the expertise to work on this. Mike might point us to the
> right direction
This is a vendor issue. Once upon a time we did a lot of digging into where the device is spending its time during this 2 seconds, and it's mostly opaque to us. The areas where we do have visibility happen quickly (think 10 to 100ms)--comparable to other platforms.
Flags: needinfo?(mhabicher)
Comment 33•11 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #29)
> To address the long waits for the camera preview on hamachi we need a
> loading bar / icon to keep the user informed about what's going on.
Hi,
I think it makes the most sense to use the spinner from our building blocks as the loading animation
http://buildingfirefoxos.com/building-blocks/progress-and-activity.html
Thanks!
Flags: needinfo?(amlee)
Comment 34•11 years ago
|
||
Amy has it correct - we should show a spinner since we can't predict how long the delay will be.
Flags: needinfo?(tshakespeare)
Assignee | ||
Comment 35•11 years ago
|
||
Comment on attachment 8406224 [details]
pull-request (master)
This needs a second review. If you would be so kind Gents... :)
Attachment #8406224 -
Flags: review?(jdarcangelo)
Attachment #8406224 -
Flags: review?(dmarcos)
Attachment #8406224 -
Flags: review+
Updated•11 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [c= p= s= u=] → [c=progress p= s= u=]
Comment 36•11 years ago
|
||
With this patch applied the camera doesn't start anymore from the lock screen with password. When tapping on the camera button the screen goes black and camera doesn't start
Comment 37•11 years ago
|
||
I made a b2gperf --delay=10 --iterations=30 camera run for both master and the branch of this bug and I didn't see any regression. Below the numbers:
MASTER
2014-04-23 16:35:04,488 B2GPerfRunner INFO | Results for camera, cold_load_time: median:1131, mean:1125, std: 24, max:1164, min:1025, all:1141,1083,1136,1138,1123,1112,1132,1155,1136,1118,1136,1131,1140,1143,1125,1113,1151,1127,1025,1142,1120,1164,1093,1124,1117,1136,1134,1116,1130,1134
PERF BRANCH
2014-04-23 16:43:37,460 B2GPerfRunner INFO | Results for camera, cold_load_time: median:1128, mean:1136, std: 40, max:1298, min:1074, all:1153,1103,1134,1153,1132,1129,1117,1298,1107,1114,1126,1115,1138,1100,1133,1122,1113,1225,1139,1152,1154,1074,1139,1099,1123,1127,1117,1168,1158,1120
Assignee | ||
Comment 38•11 years ago
|
||
dmarcos: That's good news. I'll remove startup.js then.
Comment 39•11 years ago
|
||
Comment on attachment 8406224 [details]
pull-request (master)
Considering how large this patch is, it looks like it is in really good shape. I haven't pulled it down and run it, I simply read through the code. I have a few nits that need addressed, but one in particular was a 1-line change in the SMS app that I'm not sure if it was accidental/testing. Please check and see if that was intentional before landing. Conditional R+
Attachment #8406224 -
Flags: review?(jdarcangelo) → review+
Comment 40•11 years ago
|
||
Land this puppy
Updated•11 years ago
|
Attachment #8406224 -
Flags: review?(dmarcos) → review+
Assignee | ||
Comment 41•11 years ago
|
||
Attachment #8406224 -
Attachment is obsolete: true
Comment 42•11 years ago
|
||
Wilson, The unit tests failed on travis
Comment 43•11 years ago
|
||
Fixed unit tests. Waiting for our buddy Travis to go green again!
Comment 44•11 years ago
|
||
Landed on master:
https://github.com/mozilla-b2g/gaia/commit/59f8b3723e9fc0f7d9fe838540ce5c6a9d02b2b6
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
blocking-b2g: 2.0? → 2.0+
Whiteboard: [c=progress p= s= u=] → [c=progress p= s=2014.04.25 u=2.0]
Updated•11 years ago
|
status-b2g-v2.0:
--- → fixed
Target Milestone: --- → 2.0 S1 (9may)
Updated•10 years ago
|
Attachment #8403984 -
Flags: feedback?(mhabicher)
Updated•10 years ago
|
Target Milestone: 2.0 S1 (9may) → 1.4 S6 (25apr)
Updated•10 years ago
|
Summary: [Camera][Hamachi] Long wait for initial preview stream → [Camera][Hamachi][Dolphin] Long wait for initial preview stream
Updated•10 years ago
|
status-b2g-v1.4:
--- → affected
Updated•10 years ago
|
status-b2g-v1.3T:
--- → affected
Whiteboard: [c=progress p= s=2014.04.25 u=2.0] → [c=progress p= s=2014.04.25 u=2.0][sprd315883]
Comment 47•10 years ago
|
||
Tarako is not affected by this bug. This issue only applies to the new camera implementation that landed in 1.4.
status-b2g-v1.3T:
affected → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•