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)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: romaxa, Assigned: mfinkle)

References

Details

Attachments

(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+
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
regressed tPan and tShutdown (and mfinkle said it also hurt tS) Backed out changeset fd2537bf5603
Status: RESOLVED → REOPENED
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
Status: REOPENED → RESOLVED
Closed: 15 years ago14 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: