1038179 - Contact app launch latency regressed in v2.0

Status: RESOLVED DUPLICATE of bug 1112551

(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)

Description

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

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

Comment 1

[Francisco Jordano [:arcturus] [:francisco]]

I've just sent a patch for defining the new performance events: bug 1015388

How are you meassuring this?

Comment 2

[Mike Lee [:mlee]]

David,
Can you have someone on the Contacts app team look into this?

Comment 3

[Mike Lee [:mlee]]

Here's the data we've gathered using Eideticker:


----- Forwarded Message -----
From: "William Lachance" <[e-mail]>
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]

Comment 4

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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 ?

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

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.

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

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!

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

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

[bhavana bajaj [:bajaj]]

: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.

Comment 9

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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.

Comment 10

[Francisco Jordano [:arcturus] [:francisco]]

Hi Tapas!

Yest that will be helpful!

Thanks a lot,
Francisco

Comment 11

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: 2.0patch

This is a patch that lazyload all the new features, it goes down around ~200ms.

Sergi could you provide feedback?

Comment 12

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 13

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

I forgot to add the `files` array to `LazyLoader.load` in the example in my previous comment.

Comment 14

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 16

[Francisco Jordano [:arcturus] [:francisco]]

Hi Mike,

do you know if Tapas can try this branch to check the improvements we did?

Comment 17

[Mike Lee [:mlee]]

He should be able to.

Tapas, please re-test with the branch mentioned in comment 14.

Comment 18

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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!

Comment 19

[Jean Gong :jgong]

AS mentioned in Mlee comment # 17. 
Tapas, please re-test with branch mentioned in comment 14.

Comment 20

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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.

Comment 21

[Francisco Jordano [:arcturus] [:francisco]]

Thanks!

Comment 22

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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.

Comment 23

[Francisco Jordano [:arcturus] [:francisco]]

(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!

Comment 24

[Mike Lee [:mlee]]

(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.

Comment 25

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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

[Lucas Adamski [:ladamski]]

Sounds like we're blocked on a response from Tapas?

Comment 27

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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 :) ?

Comment 28

[Francisco Jordano [:arcturus] [:francisco]]

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!

Comment 29

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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 :) .

Comment 30

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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 ?

Comment 31

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: patchv20.diff

Here is the rebased version, thanks a lot!!

Comment 32

[Francisco Jordano [:arcturus] [:francisco]]

Hi Bhavana,

We are wating for feedback to see if the patch given eases the problem.

Comment 33

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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.

Comment 34

[Francisco Jordano [:arcturus] [:francisco]]

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

Comment 35

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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

Comment 36

[bhavana bajaj [:bajaj]]

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.

Comment 37

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

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

[Inder]

Based on comment 35 and #36, removing the dependency.

Comment 39

[Brogan Zumwalt [Inactive]]

New test case needs to be written

Comment 40

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

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

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added in moztrap:

https://moztrap.mozilla.org/manage/case/14276/

Comment 42

[Francisco Jordano [:arcturus] [:francisco]]

(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?

Comment 43

[Jason Smith [:jsmith]]

(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?

Comment 44

[KTucker [:KTucker][Inactive 3/4/2016]]

Disabled the test case in moztrap.

Comment 45

[Francisco Jordano [:arcturus] [:francisco]]

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.