Closed Bug 622536 Opened 13 years ago Closed 13 years ago

OnProgress/OnStatus events processing is very expensive in Remote fennec


(Core :: IPC, defect)

Not set



Tracking Status
fennec 2.0+ ---


(Reporter: romaxa, Assigned: mfinkle)




(4 files)

After profiling bugs 622470 and 622475, I found that OnProgress/OnStatus messages processing is very expensive in latest fennec.
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.
Attached image mjit enabled svg graph
Depends on: 608956
Attached patch flip the switchSplinter Review
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)
tracking-fennec: --- → ?
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+
Closed: 13 years ago
Resolution: --- → FIXED
regressed tPan and tShutdown (and mfinkle said it also hurt tS) Backed out changeset fd2537bf5603
Resolution: FIXED → ---
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.
mfinkle also mentioned that this approach needs to be a bit more robust.
(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.
Depends on: 588441
Attachment #502046 - Flags: superreview-
Attachment #502046 - Flags: superreview+
Attachment #502046 - Flags: review?(gal)
Attachment #502046 - Flags: review-
Assignee: doug.turner → mark.finkle
tracking-fennec: ? → 2.0+
Bug 588441 landed and should have reduced the onProgress and onStatus message to zero, as shown by Doug's tests
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Great problem is not visible anymore!
I'm able to load
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.