Closed
Bug 622536
Opened 15 years ago
Closed 14 years ago
OnProgress/OnStatus events processing is very expensive in Remote fennec
Categories
(Core :: IPC, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0+ | --- |
People
(Reporter: romaxa, Assigned: mfinkle)
References
Details
Attachments
(4 files)
210.42 KB,
image/svg+xml
|
Details | |
935.10 KB,
text/plain
|
Details | |
732.76 KB,
image/svg+xml
|
Details | |
856 bytes,
patch
|
dougt
:
review-
dougt
:
superreview-
|
Details | Diff | Splinter Review |
After profiling bugs 622470 and 622475, I found that OnProgress/OnStatus messages processing is very expensive in latest fennec.
Reporter | ||
Comment 1•15 years ago
|
||
Reporter | ||
Comment 2•15 years ago
|
||
I've tested this with methodjit enabled and huge CPU consumption has gone from profile...
I think this is also slowdown performance for other pages...
Can we enable mjit here? now for fennec? we are using in maemo6 mjit ~1-2 month, and it seems work fine.
Reporter | ||
Comment 3•15 years ago
|
||
Comment 4•15 years ago
|
||
can't remove because firefox still has this pref off. Not sure what this is going to break, but I have been runnning with it and haven't noticed anything significant.
Assignee: nobody → doug.turner
Attachment #502046 -
Flags: superreview?(mark.finkle)
Attachment #502046 -
Flags: review?(gal)
Updated•15 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Comment 5•15 years ago
|
||
Comment on attachment 502046 [details] [diff] [review]
flip the switch
I'm fine with trying this again. Let's make sure we don't regress Ts.
Attachment #502046 -
Flags: superreview?(mark.finkle) → superreview+
Comment 6•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 7•15 years ago
|
||
regressed tPan and tShutdown (and mfinkle said it also hurt tS) Backed out changeset fd2537bf5603
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 8•15 years ago
|
||
mfinkle and I started debuggin more and it looks like we can greatly reduce the number of onprogress and onstatus messages that we need to process. For example, we can do something like:
diff --git a/chrome/content/bindings/browser.js b/chrome/content/bindings/browser.js
--- a/chrome/content/bindings/browser.js
+++ b/chrome/content/bindings/browser.js
@@ -4,18 +4,22 @@ let Ci = Components.interfaces;
dump("!! remote browser loaded\n")
let WebProgressListener = {
_notifyFlags: [],
_calculatedNotifyFlags: 0,
_lastLocation: null,
init: function() {
+ let flags = Ci.nsIWebProgress.NOTIFY_LOCATION |
+ Ci.nsIWebProgress.NOTIFY_SECURITY |
+ Ci.nsIWebProgress.NOTIFY_STATE_ALL;
+
let webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebProgress);
- webProgress.addProgressListener(this, Ci.nsIWebProgress.NOTIFY_ALL);
+ webProgress.addProgressListener(this, flags);
addMessageListener("WebProgress:AddProgressListener", this);
addMessageListener("WebProgress:RemoveProgressListener", this);
},
onStateChange: function onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
let webProgress = Ci.nsIWebProgressListener;
let notifyFlags = 0;
In my simple testing, it we go from around a few hundred onProgress|onStatus message for a complex page to, my favorite number, zero.
Comment 9•15 years ago
|
||
mfinkle also mentioned that this approach needs to be a bit more robust.
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9)
> mfinkle also mentioned that this approach needs to be a bit more robust.
I think I am going to push for a variation of Ben's patch in bug 588441.
Updated•14 years ago
|
Attachment #502046 -
Flags: superreview-
Attachment #502046 -
Flags: superreview+
Attachment #502046 -
Flags: review?(gal)
Attachment #502046 -
Flags: review-
Updated•14 years ago
|
Assignee: doug.turner → mark.finkle
tracking-fennec: ? → 2.0+
Assignee | ||
Comment 11•14 years ago
|
||
Bug 588441 landed and should have reduced the onProgress and onStatus message to zero, as shown by Doug's tests
Status: REOPENED → RESOLVED
Closed: 15 years ago → 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 12•14 years ago
|
||
Great problem is not visible anymore!
I'm able to load http://www.fusker.lv/index.php?lid=286341&offset=30&special=preview
with pref("image.mem.discardable", false); on fennec Maemo6
You need to log in
before you can comment on or make changes to this bug.
Description
•