535735 - [Regression] Content sink preference names need to be updated

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

Attachment: patch

In bug 481566 we set some content sink preferences so chrome interactions while pageloading were more responsive.

Those preference names were changed soon after we landed bug 481566. See bug 481566.

This patch updates the pref names in Fennec to the new versions. While testing this, I tried a few of the other preferences. See 481566 comment 9 (https://bugzilla.mozilla.org/show_bug.cgi?id=481566#c9)

I found the we had an unbalanced begin/commit batchOperations, which I tracked down to the pair in Browser.startup:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#375

If I removed those, the other preferences could be used to tweak the content sink. The problem appeared to be the addTab call near the bottom of startup. Once called, we would not return and commitBatchOperations would not be called. The load operation seemed to own the process and then got hung up somehow.

Removing the begin/commit batchOperations from startup allowed the load operation to finish and we then did return to startup to finish.

I tested Ts numbers and they were not changed by removing the batchOperations from startup.

ANYWAY.... patch

Comment 1

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

Attachment: patch 2

Doug and I talked to Jonas a bit and this new patch is the result. The important parts are:

pref("content.sink.enable_perf_mode", 1) // favor interactive
pref("content.sink.pending_event_mode", 2) // return to main loop right away
pref("content.sink.event_probe_rate", 5) // check for events every 5th html token

The other numbers are basically what we used before.

Testing shows nice panning while loading and decent load times too.

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 418335 [details] [diff] [review]
patch 2

r+ on the app/mobile.js stuff.

stuart should r+ the chrome/content/browser.js
 stuff since he spent too much time with it the other day.

Comment 3

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

Attachment: patch 3 - with semicolons

* Added semicolons to prefs :(
* No need to remove any browser.js code

Comment 4

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

pushed:
https://hg.mozilla.org/mobile-browser/rev/43c4e1d5909f

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

For posterity: If this patch caused a Tp perf regression, you should be able to lessen that perf regression by increasing the content.sink.event_probe_rate pref.

Changing content.sink.event_probe_rate from 5 to 10 should half *the Tp perf hit* (i.e. not double performance, just reduce the regression). Changing it to 20 should slash it in quarter, and so on.

Comment 6

[Aakash Desai [:aakashd]]

verified FIXED ( i can see the prefs and their defaults in about:config...plus we're faster on page loads) on builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b5pre) Gecko/20091221 Firefox/3.6b5pre Fennec/1.0b6pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.3a1pre) Gecko/20091221 Firefox/3.7a1pre Fennec/1.0b5