1059860 - Regression in contacts startup 2014-08-28

Status: RESOLVED WORKSFORME

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

Description

[William Lachance (:wlach)]

There was a noticeable regression in startup for Contacts on b2g-inbound, noticeable in both datazilla and eideticker:

Datazilla:

https://datazilla.mozilla.org/b2g/?branch=master&device=flame-319MB&range=7&test=cold_load_time&app_list=contacts&app=contacts&gaia_rev=99ed1550a810f799&gecko_rev=645a464cbf93&plot=avg

Eideticker:

http://eideticker.mozilla.org/#/b2g/flame-319/b2g-inbound/b2g-contacts-startup/timetostableframe/7

If you compare runs side-by-side in Eideticker you can see very clearly that the contacts aren't being filled in quite as fast as they were before:

Before:
http://eideticker.mozilla.org/detail.html?id=5f5b66e22e4711e4848d10ddb19eacac#/framediffsums

After: 
http://eideticker.mozilla.org/detail.html?id=b2c4f0582e5811e4babb10ddb19eacac#/framediffsums

Pushlog of changes (Gecko): https://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=5b3a96ce4225&tochange=9e899a5a311a
Pushlog of changes (Gaia): https://github.com/mozilla-b2g/gaia/compare/d281d90c3679d513627e17c0f4dcf9e895c9b47d...c366b0e21c4919a95555dfc37fe57abe41feddf2

Comment 1

[William Lachance (:wlach)]

You now need to go a bit farther back to see the eideticker regression (obviously I need date range permalinks).

http://eideticker.mozilla.org/#/b2g/flame-512/b2g-inbound/b2g-contacts-startup/timetostableframe/30

There has been no action on this bug. I can guarantee that this is going to cause problems for 2.1 acceptance. Needinfo'ing contacts owners (per https://wiki.mozilla.org/Modules/FirefoxOS) to try to get some action going.

Comment 2

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

Hi William,

thanks a lot for letting us know, we've been pretty busy with 2.1 FC, now we can take a look to this. We will schedule this ASAP.

Comment 3

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

Assigning to myself to take care of it :)

Comment 4

[Jose Manuel Cantera]

Francisco, 

let me know if you need any help with this

thanks

Comment 5

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

Jose,

right now I have a lot of things on my plate, so feel free to steal from me.

Comment 6

[Jose Manuel Cantera]

as this is not blocking and now we are swamped with blockers, reseting assignee for the moment

Comment 7

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

I've started to take a look to this problem and found the following:

26th August, we landed the new gaia-headers web component. But all apps did as well at some point
28th August, we landed the ice group to be visible on the list. The generation of the list is happening after we render the list, but we have a new template to add, that may cause a bit of increase, but not that much.

In the 8th september we can see how performance increases, but we dont have any commit on our gaia code base.

Comment 8

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

Sorry, forgot to comment that information i got from datazilla:

https://datazilla.mozilla.org/b2g/?branch=master&device=flame-319MB&range=30&test=cold_load_time&app_list=contacts&app=contacts&gaia_rev=d2c32c964016d4b2&gecko_rev=8b9750f96fd7&plot=avg

In eideticker we see kind of the same:

http://eideticker.mozilla.org/#/b2g/flame-512/b2g-inbound/b2g-contacts-startup/timetostableframe/30

A spike on the 28th August, and then by the 9th  coming back to normal results.

Comment 9

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

Hi,

I forgot to add some more extra feedback.

I've been working on loading scripts at correct time, we had some scripts that were just bein loaded at initialization that we didn't need, here is the ongoing work:

https://github.com/mozilla-b2g/gaia/pull/24334

Separated commits for easy review. Right now just asking for feedback.

This is not the cause of the regression but a enhancement.

The regresion on the 28 could be a platform one, since it went down again on the 9/9, but we had again another spike on the 9/16, which could be related to some fixed to the deferred actions (which should run after the list is painted :(  )

Comment 10

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

Attachment: WIP: Enhancement, remove not needed scripts from the critical path

Comment 11

[Jose Manuel Cantera]

Comment on attachment 8493643 [details] [review]
WIP: Enhancement, remove not needed scripts from the critical path

LGTM but I would like to do some smoke testing once you have the final PR ready for reviewing

Comment 12

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

In order to proceed on this, since the attached WIP is an improvement of the existing code I'll open a different bug and block this one.

Comment 13

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

What's the reason we have the ICE refactors in the same PR as the performance improvements?

Once this is explained, it looks good to me, I just left another small comment in the PR.

Comment 14

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

(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #13)
> What's the reason we have the ICE refactors in the same PR as the
> performance improvements?
> 
> Once this is explained, it looks good to me, I just left another small
> comment in the PR.

I've been checking the performance results and ICE doesn't add much extra, since it's happening at the end of the list rendering.

Thanks for taking a look!

Comment 15

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

I will rename this bug since the regression happening in the 28th August is part of the one which all apps have been throug (discussed in dev-gaia).

Also it got fixed around 9th September.

The one that I'm seeing is the regression in performance generated on September 16th.

Comment 16

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

Forgot to mention that this regression I'm talking about is happening in KitKat version.

Comment 17

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

I think is better to open a new bug for the new regresion and close this one as worksfor me