Closed
Bug 535735
Opened 15 years ago
Closed 15 years ago
[Regression] Content sink preference names need to be updated
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
fennec1.0
People
(Reporter: mfinkle, Assigned: mfinkle)
Details
Attachments
(1 file, 2 obsolete files)
1.28 KB,
patch
|
pavlov
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•15 years ago
|
||
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•15 years ago
|
Attachment #418335 -
Flags: review?(pavlov) → review?(mozbugz)
Comment 2•15 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+
Assignee | ||
Comment 3•15 years ago
|
||
* Added semicolons to prefs :( * No need to remove any browser.js code
Attachment #418335 -
Attachment is obsolete: true
Attachment #418337 -
Flags: review?(pavlov)
Updated•15 years ago
|
Attachment #418337 -
Flags: review?(pavlov) → review+
Assignee | ||
Comment 4•15 years ago
|
||
pushed: https://hg.mozilla.org/mobile-browser/rev/43c4e1d5909f
Status: NEW → RESOLVED
Closed: 15 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.
Comment 6•15 years ago
|
||
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.
Description
•