819091 - [meta] [Gaia::Contacts] Long initial startup time for Contacts app

Status: RESOLVED FIXED

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

Description

[Peter La]

What makes it feel slow/broken?

Long startup time for the Contacts app after a device reset/startup.  Timed consistently around 4-4.5 seconds on Unagi running Dec. 6th nightly.  David Clarke to provide numbers on Otoro.

Note this 4-4.5 seconds time is a little better than the initial 5.7s recorded in the metabug 814981.

Did it prevent you from doing what you wanted? Why?

How does this make you feel?

[ ]  :)  I feel happy about it
[ ]  :|  Meh
[X]  :(  I'm upset
[ ] >:O  I'm angry

Device: Original numbers from Unagi, Dec. 6th nightly.  David Clarke to get Otoro numbers.

Details:

B2G on Unagi:               4-4.5 seconds
Android ICS 4.0.4 on Otoro: 1.3 seconds

David Clarke to provide updated B2G on Otoro numbers.

Bonus: can you attach a video of the problem?
See metabug 814981

Comment 1

[James Lal [:lightsofapollo]]

Here is a profile...

We are spending a ton of time building the contacts list... I have a few ideas to speed things up.. 

http://people.mozilla.com/~bgirard/cleopatra/#report=43b537de27a68473cfcdb2492855458c8c22cba3

Comment 2

[[e-mail] [:onecyrenus]]

Peter, 
 some questions, do you have contacts in your phone when you arrived at these numbers ? 

i am on pto till monday, but i'll try to take a look at this first thing.

Comment 3

[James Lal [:lightsofapollo]]

Everything below is on a test run of 1k contacts. Those contacts where loaded via the UI test.

While initial loading time needs improvement I think the following probably more critical. Some of the initial loading time will improve with unrelated changes.

# The point where the user sees first contact:

- we load the sort order from asyncStorage before requesting the contacts (via the contacts api). By doing this in serial order we can up up to 1s (more/less depending on the number of contacts) before the user can even see the first contact.

# The point where user is able to scroll smoothly (when rendering chunks FPS while scrolling is between 1-10).

There are two items which really hurt the initial scrolling experience.
Time spent in building the "chunk" (the group of contacts) and reflows.

I don't have any specific suggestions to reflows yet but there are a few things we can do to reduce the time it takes to build the group of contacts.

- only escape text if it contains one or more HTML chars. This can result in huge improvements when you don't have tons of HTML text. See https://github.com/mozilla-b2g/gaia/blob/master/apps/calendar/js/template.js#L3

- in fixed_header.js#refresh we spend a ton of time here in selector
  lookups. (We spent almost 1.3S in this method alone)

- general reduction of selector lookups during chunk insertion. If you look at the profile report there is a huge amount of time spent in selector lookups and related operations. It seems possible to remove all or most lookups during insertion as they are fairly expensive and this is a high volume code path.

Comment 4

[Jose Manuel Cantera]

(In reply to James Lal [:lightsofapollo] from comment #3)
> Everything below is on a test run of 1k contacts. Those contacts where
> loaded via the UI test.
> 

> There are two items which really hurt the initial scrolling experience.
> Time spent in building the "chunk" (the group of contacts) and reflows.
> 
> I don't have any specific suggestions to reflows yet but there are a few
> things we can do to reduce the time it takes to build the group of contacts.
> 

perhaps reflow reduction can be done by using document fragments instead of adding nodes directly to the DOM. 

> - only escape text if it contains one or more HTML chars. This can result in
> huge improvements when you don't have tons of HTML text. See
> https://github.com/mozilla-b2g/gaia/blob/master/apps/calendar/js/template.
> js#L3
> 
> - in fixed_header.js#refresh we spend a ton of time here in selector
>   lookups. (We spent almost 1.3S in this method alone)
> 
> - general reduction of selector lookups during chunk insertion. If you look
> at the profile report there is a huge amount of time spent in selector
> lookups and related operations. It seems possible to remove all or most
> lookups during insertion as they are fairly expensive and this is a high
> volume code path.

Comment 5

[Jon Hylands [:jhylands]]

Looking at the profile and the code (contacts_list.js), the function showGroup() is taking 15% of the total profile time. It appears to be called once per contact (from the function cl_bc), whether the group has already been set to "not hidden" or not. With 1000 names evenly distributed, you're looking at almost 40 names per group...

Or am I completely missing something here?

Comment 6

[James Lal [:lightsofapollo]]

Re #5

Yep your correct, if you look inside that method you will see that the majority of the time spent there is in fixed_header.js#refresh (selector based lookups).

---

I wrote a completely minimal and trivial app that loads the same number of contacts and displays them (without html escaping, etc....) we obviously can't hit these same numbers but we should be able to get close if we optimize contacts application.

From my observations there is almost no visible lag when scrolling the 1k contacts in a very minimal solution so I think the overall situation is viable for this number of contacts but we need to figure out the re-flows and selector lookups.

http://people.mozilla.com/~bgirard/cleopatra/#report=7dd1d863d1e89721cff35521efeec5adfc00d6ee

https://github.com/lightsofapollo/gaia/compare/contacts-simple-hack

Comment 7

[alberto.pastor]

The code you are commenting was added 2 days ago (and Contacts loading slowness cames from time ago).

https://github.com/mozilla-b2g/gaia/commit/cf1378adba2dba6777040e7cb0e175f45979687d#apps/communications/contacts

It's true that I didn't take in account performance for that fix (I will submit a patch) but I don't think that's crucial for Contacts performance (although you are right that it affects).

The minimal contacts app you wrote, I think would be more real if we use the same markup on each contact template. I already saw that simplifying the markup to be rendered in each contact, helps a lot performance.

We can talk about all of this via IRC, as I already spend some time in checking contacts performance.

Thanks for you help guys!

Comment 8

[James Lal [:lightsofapollo]]

Another optimization we should seriously look at is loading the contacts in a non-deferred script in the header prior to CSS that will begin requesting contacts. This will let us do that in parallel to other stuff...

Comment 9

[Etienne Segonzac (:etienne)]

Alberto, do yo want to steal this to host your patch from yesterday?

Comment 10

[alberto.pastor]

Yep, I have a couple of patches, one regarding loading time (from click to first screen) and the other one improving contacts painting.
I will send both soon.

Thanks guys!

Comment 11

[Ben Francis [:benfrancis]]

This has been marked as a meta bug but doesn't yet have any dependencies, Alberto are you going to file separate bugs for each of those patches or attach them here? If you file dependent bugs please nom them with tef?

Comment 12

[James Lal [:lightsofapollo]]

Alberto, can we open up a tracking pull request for ongoing comments?

https://github.com/albertopq/gaia/tree/contacts-painting-perf (based on this)

We can bring some more profiling experts in as well who can do some more analysis.

Comment 13

[alberto.pastor]

I don't know why it has been marked as meta. Regarding the pull request, that branch is a little bit experimental. I will create another one cherry-picking the commits I know need to be added, and we can start discussing over there.

Comment 14

[alberto.pastor]

Well, I just have been asked to create different bugs for each improvement.
I will start doing that today.

Comment 15

[Ben Francis [:benfrancis]]

(In reply to James Lal [:lightsofapollo] from comment #12)
> We can bring some more profiling experts in as well who can do some more
> analysis.

Gregor and Reuben are here profiling contacts right now, they will be in touch!

Comment 16

[Dietrich Ayala (:dietrich)]

Is this less than 1.5s? Can we close this now?

Comment 17

[Gregor Wagner [:gwagner]]

(In reply to Dietrich Ayala (:dietrich) from comment #16)
> Is this less than 1.5s? Can we close this now?

No, Francisco has to land a gaia patch as well.

Comment 18

[Daniel Coloma:dcoloma]

(In reply to Gregor Wagner [:gwagner] from comment #17)
> (In reply to Dietrich Ayala (:dietrich) from comment #16)
> > Is this less than 1.5s? Can we close this now?
> 
> No, Francisco has to land a gaia patch as well.

Can we add the dependency on this bug for that gaia patch? 

Atul, we do not block on meta-bugs as this one, I'd suggest you mark the child bugs as blockers if you deem they are critical.

Comment 19

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

Just added the bug 848188 as a blocker of this, that is a WIP, as we reduce the timing a lot, we are lagging the scroll at the beginning as well.

Already ask Alberto to take a look to see if we can improve that.

Cheers.

Comment 20

[Ben Kelly [:bkelly, not reviewing]]

What is the status of this bug?  It seems that Francisco landed the patch people were waiting for in comment 19.  Should this be closed?

Alternatively, I am now working contacts launch latency in bug 871823.  Should that be added as a dependency?

Comment 21

[Kevin Grandon :kgrandon]

Closing this bug in favor of working in bug 871823 for now. If there's anything else to do here please reopen or open blocking bugs against 871823.