942397 - image_loader.js onScroll() reduces scroll fps due to timeout calls

Status: RESOLVED FIXED

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

Description

[Ben Kelly [:bkelly, not reviewing]]

While investigating bug 937425 for reduced scrolling fps in the contacts app, I noticed that our onscroll event handlers were delaying the layout process too much.  In particular, the imgLoader.onScroll() function appeared to be a problem.  This code essentially does this on every scroll event:

  clearTimeout(scrollTimer);
  scrollTimer = setTimeout(update, 100);

Generally clearing and setting timers is no a problem, but in this case its a bit too slow to call for every scroll event.

I have a patch that simply waits for the scroll timer to fire and then checks to see how long it has been since the last scroll event.  If necessary, it then fires another timer.  This helps throttle our setTimeout() calls to once every 100ms.

Patch to follow.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

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

Jose, this pull request limits how often the imgLoader calls setTimeout() in order to optimize our scroll handlers and improve fps.  It does this by putting some timestamp math in the timeout handler and triggering another delay if necessary.

Note, I have tested this with the main list, but I don't have a working FB configuration since my firmware has broken wifi at the moment.  Can you verify that it works with FB?  Since its a timer only change, though, I don't see why it would break with FB.

Thank you!

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Nominating for koi? because the risk is low and this has a significant fps impact for bug 937425.

Comment 3

[Jose Manuel Cantera]

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

Redirecting the review to Cristian as he created the image loader module originally

thanks!

Comment 4

[Cristian Rodriguez (:crdlc)]

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

The code looks good and it is an excellent idea in my honest opinion. I added a comment with a suggestion if you want to implement, thanks a lot

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Thanks for the review!  I agree your code was easier to read so I updated the PR to use it.  Travis is green, so landed:

  https://github.com/mozilla-b2g/gaia/commit/4cc4c2bf30c6451eacd588e6dfabca2458a0e62f

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Uplifted to v1.2:  e10219d360adcf78efdfb01c71c3fa8d1acc239e