632072 - Do not create new functions for every kinetic pan

Status: RESOLVED WONTFIX

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

Description

[Benjamin Stover (:stechz)]

Due to paranoia from bug 631951, I was worried that we may be compiling Javascript functions for every kinetic pan we were making when we turn methodjit on. It was faster to make a cleanup patch than asking anyone if this could be problematic. :)

Comment 1

[Benjamin Stover (:stechz)]

Attachment: Do not create new functions for every kinetic pan

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

This patch was badly bit-rotted.  This updates Ben's approach to work against the current code, and again prevent us from creating new objects or functions on each kinetic pan.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2 (diff -w)

Here's a "diff -w" output, to make it easier to separate the real changes from the indentation changes.

Comment 4

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

Comment on attachment 565128 [details] [diff] [review]
patch v2

The priginal worry was related to methodjit, which is off in chrome now. Inany case, this shouldn't hurt.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: s/self/this/g

This mechanical patch removes the now-unnecessary "self" local and replaces it with "this" inside onBeforePaint.  I'll fold this into the main patch before pushing it; just keeping it separate for now to keep reviews easier.

Comment 6

[Matt Brubeck (:mbrubeck)]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> The priginal worry was related to methodjit, which is off in chrome now.
> Inany case, this shouldn't hurt.

True, though this should at least reduce the amount of garbage collection we do.  It will also help if we manage to fix the methodjit memory usage enough to turn it back on.

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac1d5e94097a Thanks Ben!

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ac1d5e94097a

Comment 9

[Matt Brubeck (:mbrubeck)]

Backed out because this broke kinetic panning on repeated swipes:
https://hg.mozilla.org/mozilla-central/rev/737e0ebf68b2

I'll go back and see what I messed up...

Comment 10

[Matt Brubeck (:mbrubeck)]

This code is going away in favor of AsyncPanZoomController.