901657 - contacts merge progress animation using CPU when hidden

Status: RESOLVED FIXED

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

Description

[Ben Kelly [:bkelly, not reviewing]]

I noticed today that our time to load the full reference-workload-heavy contact list went from less than 8.5 seconds up to over 9.7 seconds.  Git bisecting showed that this was due to :

  https://github.com/mozilla-b2g/gaia/commit/d6567edc44dea923914935b1f256c4fce67856fd

Looking at the code I don't see an obvious explanation for this.  As far as I can tell all of these changes are outside of the contact list.  We'll need to investigate further.  This may be a requirement of the new feature or perhaps its something we can work around.

I'll post a patch with the code I am using to measure the time.  I'd really like to setup an automated system to monitor the time so we can proactively detect changes.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: contacts_timing.patch

This patch outputs times to logcat for both the above-the-fold event and full load from contactsAPI.  The above-the-fold time is obviously important for showing a useable UI to the user quickly.  The full load time dictates how soon we can use the letter group shortcuts to jump to the end of the list.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

It appears that the app container is using a steady 25% CPU when idle after this change.  Still need to investigate more.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

This appears to be due to the pack-activity class animation running in the background.  This occurs because the #throbber progress element is using visibility:hidden instead of display:none.

It seems there are two straightforward ways to handle this.  Either switch to using display:none or only add the pack-activity class when the progress element is displayed.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Pull request at https://github.com/mozilla-b2g/gaia/pull/11377

This pull request fixes the issue by using the standard hide class instead of visibility:hidden.

Jose, requesting review from you because I believe Cristian is still on holiday and it would be nice to get this fixed soon.

Thanks!

Comment 5

[Jose Manuel Cantera]

Comment on attachment 786288 [details]
Pull request at https://github.com/mozilla-b2g/gaia/pull/11377

good work and good catch!

thanks

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Thanks Jose!  Merged:

  https://github.com/mozilla-b2g/gaia/commit/a3630e49320424d63f5de0c57d48fde1494569b2