Closed
Bug 1038179
Opened 10 years ago
Closed 10 years ago
Contact app launch latency regressed in v2.0
Categories
(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)
Tracking
(tracking-b2g:backlog)
RESOLVED
DUPLICATE
of bug 1112551
tracking-b2g | backlog |
People
(Reporter: tkundu, Assigned: arcturus)
References
Details
(Keywords: perf, regression, Whiteboard: [caf priority: p2][c=regression p= s= u=2.0] [CR 694011] [priority])
Attachments
(1 file, 1 obsolete file)
4.64 KB,
patch
|
Details | Diff | Splinter Review |
Contact app launch latency has regressed compared to v1.3.
For v1.3:
gaia: https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/commit/?h=mozilla/v1.3&id=e08beb0297aff472b5c84437e9d7eaf8c0400a29
gecko: https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/v1.3&id=e0bfab94fd20e4dcbb2fecf19d7c2dc606189f81
For v2.0:
gaia: https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/commit/?h=mozilla/v2.0&id=ef67af27dff3130d41a9139d6ae7ed640c34d922
gecko: https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/v2.0&id=d3eae03cdf4e6944e646d05938a22dc1380a0d95
Test Steps:
1) Flush v1.3 and v2.0 build on two flame device (512MB) device and load 200 contacts on both device. Launch contact app and compare side by side.
2) launch latency = touchdown to displaying settings options
In v1.3 launch latency is : 944 msecs
In v2.0 launch latency is : 1377 msecs
Reporter | ||
Updated•10 years ago
|
blocking-b2g: --- → 2.0?
Reporter | ||
Updated•10 years ago
|
Whiteboard: [CR 694011]
Assignee | ||
Comment 1•10 years ago
|
||
I've just sent a patch for defining the new performance events: bug 1015388
How are you meassuring this?
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 2•10 years ago
|
||
David,
Can you have someone on the Contacts app team look into this?
Flags: needinfo?(mlee) → needinfo?(dscravaglieri)
Whiteboard: [CR 694011] → [c=regression p= s= u=2.0] [CR 694011]
Comment 3•10 years ago
|
||
Here's the data we've gathered using Eideticker:
----- Forwarded Message -----
From: "William Lachance" <wlachance@mozilla.com>
Subject: Re: Regressions in App startup time in 2.0
Ok did a run, results below. Videos, etc. at
http://people.mozilla.org/~wlachance/eideticker-startup-test/
(click through to the index page, then click on the bars for videos and an
option to dig into the detail of what we're measuring... e.g.
http://people.mozilla.org/~wlachance/eideticker-startup-test/14-results/contacts-startup-14/#/timetostableframe
->
http://people.mozilla.org/~wlachance/eideticker-startup-test/14-results/contacts-startup-14/detail.html?id=90d314340b8f11e4b65210ddb19eacac#/framediffsums)
Remember this is a comparison between 1.4 and 2.0, so not the same as
what Qualcomm was measuring.
Still:
1. We see a small regression with contacts
2. An *improvement* in settings
3. An *improvement* in camera.
So not what they were seeing.
Hope this helps and let me know if you need more help interpreting this
data,
Will
----
v2.0 aurora (build on 14 July):
=== Results on b2g-contacts-startup for FirefoxOS ===
Times to first stable frame (seconds):
[1.8833333333333333, 1.95, 1.9, 1.9833333333333334, 1.8666666666666667]
=== Results on b2g-messages-startup for FirefoxOS ===
Times to first stable frame (seconds):
[4.683333333333334, 4.466666666666667, 4.633333333333334,
4.466666666666667, 4.55]
=== Results on b2g-settings-startup for FirefoxOS ===
Times to first stable frame (seconds):
[2.316666666666667, 2.1333333333333333, 2.2666666666666666, 2.1,
2.283333333333333]
=== Results on b2g-camera-startup for FirefoxOS ===
Times to first stable frame (seconds):
[2.6333333333333333, 2.2333333333333334, 2.95, 2.966666666666667, 2.2]
v1.4 (build on 14 July):
=== Results on b2g-contacts-startup for FirefoxOS ===
Times to first stable frame (seconds):
[1.8, 1.65, 1.65, 1.8166666666666667, 1.5666666666666667]
=== Results on b2g-messages-startup for FirefoxOS ===
Times to first stable frame (seconds):
[4.016666666666667, 4.0, 4.133333333333334, 4.133333333333334,
3.9833333333333334]
=== Results on b2g-settings-startup for FirefoxOS ===
Times to first stable frame (seconds):
[2.6166666666666667, 2.5, 2.8666666666666667, 2.1333333333333333, 2.6]
=== Results on b2g-camera-startup for FirefoxOS ===
Times to first stable frame (seconds):
[3.3, 2.933333333333333, 3.0, 2.8833333333333333, 2.816666666666667]
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #1)
> I've just sent a patch for defining the new performance events: bug 1015388
>
> How are you meassuring this?
We measured using high speed camera. Could you please follow the steps which I mentioned in Comment 0 ?
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(francisco)
Assignee | ||
Comment 5•10 years ago
|
||
Hi,
so we are talking about the latency on displaying the settings.
I'll need more info, since we are doing more operations, is the list completely loaded, or are you launching settings while the list is still being loaeded, not even displaying a contact, etc.
Flags: needinfo?(tkundu)
Flags: needinfo?(francisco)
Flags: needinfo?(dscravaglieri)
Assignee | ||
Comment 6•10 years ago
|
||
Hi folks we are taking a look to this and we have some issues understanding the problems:
- Are we taking about a regression from 1.3 to 2.0 or 1.4 to 2.0 (which makes sense to me)
- When talking about startup time, util we see the settings icon isnt?
Thanks a lot folks!
Assignee | ||
Comment 7•10 years ago
|
||
Hi again,
went through codeb base for 1.4 and 2.0 and the comparisson is the following:
- In 2.0 we got the extra stuff:
- New css file for the edit mode (this is a feature in 2.0): /shared/style/edit_mode.css
- New l10n refactor (this happened for all apps)
- Also a global change in all apps font_size_utils.js added to calculate font header
- /shared/js/mime_mapper.js this one we can remove it from the critical path and lazyload, is a 4kb file.
- /shared/locales/import_contacts.ini another locale file, but during build process should not affect.
- New feature: nfc so we have to load: contacts/js/nfc.js
Perhaps that could explain the small regression between 1.4 and 2.0
When we compare 1.3 with 2.0, a part from the previous changes, we also find the following in the critical path:
-We landed 1.4 features dsds so we got the following extra stuff from 1.3 to 2.0:
- shared/style/sim_picker.css
- locales for sim picker (as they are shared), but shouldn't affect after the build process.
I don't have a speed camera but I've been hacking homescreen app and contacts app for doing the following:
- Output Date.now() just before we execute app.launch in the homescreen
- Output Date.now() when we remove the 'hidden' class from the body of the app.
Of course it's not meassuring the time gecko needs to repaint the whole app but the times that I got are the following:
1.4 ~ 780 ms
2.0 ~ 880ms
2.1 ~ 870ms
All of that meassured with 500 contacts. Couldn't get 1.3 data, since we don't have builds for 1.3 and don't think that makes sense to compare gecko 1.4 with gaia 1.3.
The data that I got, kind of match the eideticket data.
Tapas could we have another meassure?
Comment 8•10 years ago
|
||
:francisco, since you are helping with investigation here, I'll set the assignee to you as we do not want to have unassigned blockers. Feel free to reassign as needed.
Assignee: nobody → francisco
Reporter | ||
Comment 9•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #6)
> Hi folks we are taking a look to this and we have some issues understanding
> the problems:
>
> - Are we taking about a regression from 1.3 to 2.0 or 1.4 to 2.0 (which
> makes sense to me)
> - When talking about startup time, util we see the settings icon isnt?
>
> Thanks a lot folks!
We compared contact app launch for same 200 contacts loaded in both v1.3 and v2.0 512MB device.
here launch latency means 'touch down to displaying full page of contacts'
My understanding is that 'end user' will feel bad if they see big delay after moving from v1.3 to v2.0 (see comment 0 for exact numbers) .
I can share a side by side contact app launch comparison video for v1.3 and v2.0. Please let me know if it will help you here.
Flags: needinfo?(tkundu) → needinfo?(francisco)
Assignee | ||
Comment 10•10 years ago
|
||
Hi Tapas!
Yest that will be helpful!
Thanks a lot,
Francisco
Flags: needinfo?(francisco) → needinfo?(tkundu)
Assignee | ||
Comment 11•10 years ago
|
||
This is a patch that lazyload all the new features, it goes down around ~200ms.
Sergi could you provide feedback?
Attachment #8458069 -
Flags: feedback?(sergi.mansilla)
Comment 12•10 years ago
|
||
Comment on attachment 8458069 [details]
2.0patch
I think that you might want to keep the check for NFC. Something like that should work:
LazyLoader.load(['/shared/style/sim_picker.css',
'/shared/style/action_menu.css'], function() {
var files = [simPickerNode,
'/dialer/js/telephony_helper.js',
'/shared/js/contacts/utilities/templates.js'
];
if ('mozNfc' in navigator) {
files.push('/contacts/js/nfc.js');
}
LazyLoader.load(, function() {
navigator.mozL10n.translate(simPickerNode);
detailsReady = true;
contactsDetails = contacts.Details;
contactsDetails.init();
callback();
});
});
Everything else looks good.
Attachment #8458069 -
Flags: feedback?(sergi.mansilla) → feedback+
Comment 13•10 years ago
|
||
I forgot to add the `files` array to `LazyLoader.load` in the example in my previous comment.
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 14•10 years ago
|
||
Hi,
A branch with Francisco's patch that lazy-loads all new features is here: https://github.com/sergi/gaia/tree/bug_1038179
Comment 15•10 years ago
|
||
Tapas, is it easy to run your test against this branch and see the difference? If that's a problem, the PR will eventually get into the 2.0 codebase and you can retest v2.0.
Assignee | ||
Comment 16•10 years ago
|
||
Hi Mike,
do you know if Tapas can try this branch to check the improvements we did?
Flags: needinfo?(mlee)
Comment 17•10 years ago
|
||
He should be able to.
Tapas, please re-test with the branch mentioned in comment 14.
Flags: needinfo?(mlee)
Comment 18•10 years ago
|
||
Hi,
Just a reminder that this is a blocker issue and we would really appreciate timely feedback about whether the fix improves things in the speed camera test. This is Contact's team current main concern and we are looking forward to closing this issue :)
Thanks!
Blocks: CAF-v2.0-CC-metabug
No longer blocks: CAF-v2.0-FC-metabug
Comment 19•10 years ago
|
||
AS mentioned in Mlee comment # 17.
Tapas, please re-test with branch mentioned in comment 14.
Reporter | ||
Comment 20•10 years ago
|
||
(In reply to Jean Gong from comment #19)
> AS mentioned in Mlee comment # 17.
> Tapas, please re-test with branch mentioned in comment 14.
Sorry for delayed reply. Thanks a lot for your help. It seems to me that I just need to cherry-pick following patch[1] into FFOS 2.0 gaia branch [2]
[1] https://github.com/sergi/gaia/commit/c43184f85a3d13c081591011e27342bbbea2435f
[2] https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/log/?h=mozilla/v2.0
I am testing it now and I will update you soon.
Assignee | ||
Comment 21•10 years ago
|
||
Thanks!
Reporter | ||
Comment 22•10 years ago
|
||
(In reply to Mike Lee [:mlee] Paris: 2014.07.21 - 25 from comment #17)
> He should be able to.
>
> Tapas, please re-test with the branch mentioned in comment 14.
I shared some profiling data with you by email. I am still seeing regression in launch latency even with this patch.
Flags: needinfo?(tkundu)
Assignee | ||
Comment 23•10 years ago
|
||
(In reply to Tapas Kumar Kundu from comment #22)
> (In reply to Mike Lee [:mlee] Paris: 2014.07.21 - 25 from comment #17)
> > He should be able to.
> >
> > Tapas, please re-test with the branch mentioned in comment 14.
>
> I shared some profiling data with you by email. I am still seeing regression
> in launch latency even with this patch.
Can Sergi and myself have access to that profiling information?
Thanks in advance!
Updated•10 years ago
|
Whiteboard: [c=regression p= s= u=2.0] [CR 694011] → [caf priority: p2][c=regression p= s= u=2.0] [CR 694011]
Updated•10 years ago
|
Keywords: regression
Updated•10 years ago
|
Blocks: CAF-v2.0-FC-metabug
Comment 24•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #23)
>
> Can Sergi and myself have access to that profiling information?
>
Forwarded you the email with attachment from Tapas |July 22, 2014 5:34 PM| Paris time. Per Tapas' request please don't attach the video to this bug.
No longer blocks: CAF-v2.0-FC-metabug
Comment 25•10 years ago
|
||
Hi,
Thanks for the video. We are a bit puzzled though because the speed-camera measurements you provided before were very precise and the last ones you sent in the email look rough, since they are made by hand (is there anything else than the video?).
Since we know that the Homescreen is adding part of the delay, we can't take this video as a proof that contacts is still slow. Another speed-camera measurement would help a lot in determining where the bottleneck is.
Thanks!
Comment 26•10 years ago
|
||
Sounds like we're blocked on a response from Tapas?
Flags: needinfo?(tkundu)
Reporter | ||
Comment 27•10 years ago
|
||
(In reply to Sergi Mansilla (Telenor) from comment #25)
> Hi,
>
> Thanks for the video. We are a bit puzzled though because the speed-camera
> measurements you provided before were very precise and the last ones you
> sent in the email look rough, since they are made by hand (is there anything
> else than the video?).
>
The video which I shared by via last email has low resolution ( I compressed it to decrease video file size) . But it clearly shows delay between contact app launch latency in v2.0 and v1.3.
> Since we know that the Homescreen is adding part of the delay, we can't take
> this video as a proof that contacts is still slow.
I didn't understand how homescreen is adding part of contact app launch delay. Could you please help me to understand it :) ?
Flags: needinfo?(tkundu) → needinfo?(sergi.mansilla)
Assignee | ||
Comment 28•10 years ago
|
||
Hi Tapas,
we are asking for numbers, as we got in comment 0, cause with this patch we hope that we reduced the time around 100ms, that mixed with the fact that the homescreen is adding an additional delay, perhaps the combination of both is causing this regression that you are commenting.
We got this info from you:
In v2.0 launch latency is : 1377 msecs
Can you tell us how much latency you get with the patch applied?
Thanks a lot!
Flags: needinfo?(tkundu)
Reporter | ||
Comment 29•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #28)
> Hi Tapas,
>
> we are asking for numbers, as we got in comment 0, cause with this patch we
> hope that we reduced the time around 100ms, that mixed with the fact that
> the homescreen is adding an additional delay, perhaps the combination of
> both is causing this regression that you are commenting.
>
> We got this info from you:
> In v2.0 launch latency is : 1377 msecs
>
> Can you tell us how much latency you get with the patch applied?
>
> Thanks a lot!
I will confirm you soon. Thanks for the clarification :) .
Reporter | ||
Comment 30•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #28)
> Hi Tapas,
>
> we are asking for numbers, as we got in comment 0, cause with this patch we
> hope that we reduced the time around 100ms,
Could you please rebase this patch for v2.0 tip again ?
Flags: needinfo?(tkundu) → needinfo?(francisco)
Assignee | ||
Comment 31•10 years ago
|
||
Here is the rebased version, thanks a lot!!
Attachment #8458069 -
Attachment is obsolete: true
Flags: needinfo?(francisco) → needinfo?(tkundu)
Updated•10 years ago
|
Flags: needinfo?(sergi.mansilla)
Assignee | ||
Comment 32•10 years ago
|
||
Hi Bhavana,
We are wating for feedback to see if the patch given eases the problem.
Flags: needinfo?(bbajaj)
Reporter | ||
Comment 33•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #32)
> Hi Bhavana,
>
> We are wating for feedback to see if the patch given eases the problem.
Sorry for the delayed response here. We tried to test it last week but we missed last high fps camera launch latency measurement cycle because patch was not re-based for v2.0 (See Comment 30)
I am following up with our internal launch latency measurement team for this patch and we will definitely update you asap. Please expect response from us by Friday, 8th Aug.
Assignee | ||
Comment 34•10 years ago
|
||
Hi all,
I'll be on PTO starting tomorrow till 14th August, meanwhile Jose or Sergi could help with this.
I just checked that patch applies perfectly to 2.0 branch, so hopefully that won't be a problem.
Thanks
Reporter | ||
Comment 35•10 years ago
|
||
(In reply to Francisco Jordano [:arcturus] [:francisco] (on PTO till 14th August) from comment #31)
> Created attachment 8466127 [details] [diff] [review]
> patchv20.diff
>
> Here is the rebased version, thanks a lot!!
build[1] Contact app launch latency with this patch : 1453ms
build[1] Contact app launch latency without this patch : 1411ms
So we are not seeing any improvement with this patch (attachment 8466127 [details] [diff] [review]) if we compare this number to Comment 0.
BTW, from Comment 0, we are seeing 1411-944=467 ms regression in FFOS v2.0 compared to v1.3
But in Android KK, Contact app is showing launch latency as 1342ms .
So if mozilla does not want to improve launch latency regression between v2.0 and v1.3 then we can consider it as completed. It is not blocking from us anymore. Sorry for late confirmation.
[1] https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/commit/?h=mozilla/v2.0&id=9e5907995c9327f14cb5d182cee5ff16b1743ed4
https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/v2.0&id=ed87f0a54baf646eb0b20b4debc090c8016d2104
Device : msm8610 512MB
Flags: needinfo?(tkundu) → needinfo?(sergi.mansilla)
Comment 36•10 years ago
|
||
Clearing my NI as we've got feedback from CAF here.
Franciso, given comment #35 I think we should be ok to close this issue if the latency is acceptable to CAF.
But, looks like the launch latency gets worse with the patch applied, so we may not want to land it on master. Franciso, :geo should be able to help us with instructions on set-up so you are able to test this locally.
Flags: needinfo?(bbajaj) → needinfo?(francisco)
Updated•10 years ago
|
Flags: needinfo?(gmealer)
Yes, make test-perf should be up to doing pre/post-patch tests locally at this point, but I'm doublechecking accuracy and pulling instructions together. Will follow up later today.
Comment 38•10 years ago
|
||
Based on comment 35 and #36, removing the dependency.
No longer blocks: CAF-v2.0-CC-metabug
Updated•10 years ago
|
Flags: in-moztrap?(bzumwalt)
Comment 39•10 years ago
|
||
New test case needs to be written
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
I'll handle the make test-perf comparison on this one, re: comment 36. I should be able to turn numbers around tomorrow.
Re: comment 39, we probably need to handle performance bugs a little differently than functional blockers. jsmith will follow up offline, but in this case I can say that we already have tests for contact startup defined in automation and a moztrap case likely isn't necessary.
Comment 41•10 years ago
|
||
Test case added in moztrap:
https://moztrap.mozilla.org/manage/case/14276/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(bzumwalt)
Flags: in-moztrap+
Assignee | ||
Comment 42•10 years ago
|
||
(In reply to bhavana bajaj [:bajaj] from comment #36)
> Clearing my NI as we've got feedback from CAF here.
>
> Franciso, given comment #35 I think we should be ok to close this issue if
> the latency is acceptable to CAF.
>
> But, looks like the launch latency gets worse with the patch applied, so we
> may not want to land it on master. Franciso, :geo should be able to help us
> with instructions on set-up so you are able to test this locally.
I would prefer to leave this open, but remove the 2.0+ since it's not a blocker anymore. And I really want to fix this.
Regarding the latency increasing with this patch ... it's really suprising, we have been doing tests and we see around 100ms of improvement.
Wesley, should we remove the 2.0+ then based on comments 35 and 36?
Flags: needinfo?(francisco) → needinfo?(whuang)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][2.0-signoff-need+]
Updated•10 years ago
|
blocking-b2g: 2.0+ → backlog
Flags: needinfo?(whuang)
Whiteboard: [caf priority: p2][c=regression p= s= u=2.0] [CR 694011] → [caf priority: p2][c=regression p= s= u=2.0] [CR 694011] [priority]
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage+][2.0-signoff-need+] → [QAnalyst-Triage+]
Comment 43•10 years ago
|
||
(In reply to KTucker (:KTucker) from comment #41)
> Test case added in moztrap:
>
> https://moztrap.mozilla.org/manage/case/14276/
We probably should be relying on automation here, rather than having a manual test case here, given that need reliability of consistent measurements to determine the performance here. Can we disable this test case?
Flags: needinfo?(ktucker)
Comment 44•10 years ago
|
||
Disabled the test case in moztrap.
Flags: needinfo?(ktucker)
Flags: in-moztrap-
Flags: in-moztrap+
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(sergi.mansilla)
Flags: needinfo?(gmealer)
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
Assignee | ||
Comment 45•10 years ago
|
||
I think that after landing bug 1112551 that will make it to 2.2, we can close this bug since the launch numbers that we get are below the expected ones.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•