621984 - replace mozRequestAnimationFrame() with mozRequestAnimationFrame(callback)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Oleg Romashin (:romaxa)]

discussion on irc about scrolling performance related problems in fennec
**************************
23:16 < romaxa> roc: I'm not 100% sure yet and don't have enough profile data, but I've commented out this 
                http://hg.mozilla.org/mobile-browser/file/e600436e7e97/chrome/content/input.js#l390 ... do you think it is cheap to call 
                mozRequestAnimationFrame and addEventListener("MozBeforePaint") while scrolling?
23:17 < romaxa> roc:: and removing that give us ~2-3 FPS (56->59)
23:18 < roc> you should use mozRequestAnimationFrame(callback) instead of the event listener, that'd be faster
**************************

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

Uses callback listener instead of events

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 500800 [details] [diff] [review]
patch

Seems good to me.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/27cd61374fac

Comment 4

[Martijn Wargers (dead)]

I'm not seeing a noticeable difference in tpan scores. Not sure if that's expected, though:
http://graphs.mozilla.org/graph.html#tests=[[69,11,469]]&sel=1293806051,1294632364
http://graphs.mozilla.org/graph.html#tests=[[69,11,622]]&sel=1293264597,1294634165

Comment 5

[Martijn Wargers (dead)]

Not sure if there is any other way to verify this fix other than code inspection?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #5)
> Not sure if there is any other way to verify this fix other than code
> inspection?

Probably not. I don't know if Tpan tests kinetic panning or not. Even so, we made the change because:
* Robert (roc) thought it would be faster. How much faster? we don't know.
* It's cleaner. We don't need to worry about removing the event listener.

Comment 7

[Oleg Romashin (:romaxa)]

would be nice to check Tpan results with commented out 
this._waitingForPaint = true

Comment 8

[Martijn Wargers (dead)]

(In reply to comment #7)
> would be nice to check Tpan results with commented out 
> this._waitingForPaint = true

What does that do?
Is there a way to do the Tpan test locally?

Comment 9

[Andreea Pod]

Can somebody verify this please?