[Regression] Content sink preference names need to be updated

VERIFIED FIXED in fennec1.0

Status

Fennec Graveyard
General
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

Trunk
fennec1.0
x86
Linux

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 418332 [details] [diff] [review]
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
Attachment #418332 - Flags: review?(pavlov)
Created attachment 418335 [details] [diff] [review]
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.
Assignee: nobody → mark.finkle
Attachment #418332 - Attachment is obsolete: true
Attachment #418335 - Flags: review?(pavlov)
Attachment #418332 - Flags: review?(pavlov)
(Assignee)

Updated

8 years ago
Attachment #418335 - Flags: review?(pavlov) → review?(mozbugz)

Comment 2

8 years ago
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.
Attachment #418335 - Flags: review?(mozbugz) → review+
Created attachment 418337 [details] [diff] [review]
patch 3 - with semicolons 

* Added semicolons to prefs :(
* No need to remove any browser.js code
Attachment #418335 - Attachment is obsolete: true
Attachment #418337 - Flags: review?(pavlov)

Updated

8 years ago
Attachment #418337 - Flags: review?(pavlov) → review+
pushed:
https://hg.mozilla.org/mobile-browser/rev/43c4e1d5909f
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → RC
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.
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.