Closed Bug 655647 Opened 14 years ago Closed 13 years ago

Allow per-process GC/CC from about:memory

Categories

(Toolkit :: about:memory, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: n.nethercote, Assigned: deLta30)

References

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file, 3 obsolete files)

Bug 654041 added GC and CC buttons to about:memory that work for the main process. This is currently fine on desktop but doesn't allow the content process to be GC'd on Fennec. We should change this by adding GC and CC buttons for every process in about:memory. Bug 633305 serves as a good guide for this bug.
Depends on: 654041
Assignee: nnethercote → nobody
Component: General → about:memory
QA Contact: general → about.memory
I will be attempting to solve this bug.This would be a good start for me.
Awesome. You might want to join #developers or #content on our IRC server to talk to people who know about the multiprocess setup.
Proposed approach: 1) Implementing sendAsyncMessage() method in http://mxr.mozilla.org/mozilla- central/source/toolkit/components/aboutmemory/content/aboutMemory.js and addMessageListener() method in http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/content.js.
2) When "GC" or "CC" buttons are clicked on memory page,a message is sent to http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/content.js and it is received by addMessageListener() method which will trigger methods with same functionality as defined in http://mxr.mozilla.org/mozilla-central/source/toolkit/components/aboutmemory/content/aboutMemory.js#95 and http://mxr.mozilla.org/mozilla-central/source/toolkit/components/aboutmemory/content/aboutMemory.js#101. Suggestions?
dougt, IIRC you implemented the multi-process support in about:memory -- how does comment 3 and comment 4 sound to you?
Whiteboard: [MemShrink]
That's pretty specific to about:memory. Do you think there is ever going to be a need to cause a GC/CC from any where else? If so, maybe we should hang a "trigger GC/CC" method on PBrowser?
I think adding an IPDL message is overkill at this point. In general, we try to avoid triggering GC or CC explicitly, and I'm having difficulties imagining when this message would be used outside of this instance. I think going the route of loading a content script and sending messages is the way to go here.
Why isn't adding an IPDL message a good idea?
Because it makes it more likely that other consumers will try to use it as well. We don't want people to be triggering memory management tasks whenever they want.
jdm is probably right, we can always promote a new message. also, the way that this will be built is that chrome code will probably be able to send a message (via the observer service) that the child will see and instruct that child to gc/cc. at least that is how the ipc memory report code works now.
Whiteboard: [MemShrink] → [MemShrink:P2]
review?
Need test cases.
Comment on attachment 559198 [details] [diff] [review] Added messaging mechanism to send messages to content processes Jiten, if you want to request review on a patch, click on its "details" link, then select '?' in the "review" drop-down box, then enter the name of a person to do the review. In this case, it sounds like :dougt is a good person to ask for review.
Attachment #559198 - Flags: review?(doug.turner)
Comment on attachment 559198 [details] [diff] [review] Added messaging mechanism to send messages to content processes Review of attachment 559198 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. just need to rename the observer topics. Lets see another patch, getting close. ::: dom/ipc/ContentParent.cpp @@ +199,5 @@ > obs->AddObserver(this, "xpcom-shutdown", PR_FALSE); > obs->AddObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC, PR_FALSE); > obs->AddObserver(this, "child-memory-reporter-request", PR_FALSE); > obs->AddObserver(this, "memory-pressure", PR_FALSE); > + obs->AddObserver(this, "doGC", PR_FALSE); child-gc-request @@ +200,5 @@ > obs->AddObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC, PR_FALSE); > obs->AddObserver(this, "child-memory-reporter-request", PR_FALSE); > obs->AddObserver(this, "memory-pressure", PR_FALSE); > + obs->AddObserver(this, "doGC", PR_FALSE); > + obs->AddObserver(this, "doCC", PR_FALSE); child-cc-request @@ +291,5 @@ > obs->RemoveObserver(static_cast<nsIObserver*>(this), "memory-pressure"); > obs->RemoveObserver(static_cast<nsIObserver*>(this), "child-memory-reporter-request"); > obs->RemoveObserver(static_cast<nsIObserver*>(this), NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC); > + obs->RemoveObserver(static_cast<nsIObserver*>(this), "doGC"); > + obs->RemoveObserver(static_cast<nsIObserver*>(this), "doCC"); same - rename these too. ::: toolkit/components/aboutmemory/content/aboutMemory.js @@ +96,5 @@ > { > Cu.forceGC(); > + var os = Cc["@mozilla.org/observer-service;1"] > + .getService(Ci.nsIObserverService); > + os.notifyObservers(null, "doGC", null); white space is off here. @@ +107,5 @@ > .getInterface(Ci.nsIDOMWindowUtils) > .cycleCollect(); > + var os = Cc["@mozilla.org/observer-service;1"] > + .getService(Ci.nsIObserverService); > + os.notifyObservers(null, "doCC", null); same ws problem
Attachment #559198 - Flags: review?(doug.turner) → review-
Attached patch Made changes according to suggestions (obsolete) (deleted) — Splinter Review
Attachment #559752 - Flags: review?(doug.turner)
Attachment #559752 - Attachment is obsolete: true
Attachment #559752 - Flags: review?(doug.turner)
Attachment #559759 - Flags: review?(doug.turner)
Attachment #559198 - Attachment is obsolete: true
Comment on attachment 559759 [details] [diff] [review] Sorry,slight changes were left.Review this one,not the last one. This looks okay, but whitespace is off in ContentChild, and in aboutMemory.js
Attached patch Fixed whitespace (deleted) — Splinter Review
I have set whitespace at the starting of the line, according to the code above it.
Attachment #559759 - Attachment is obsolete: true
Attachment #559759 - Flags: review?(doug.turner)
Attachment #559800 - Flags: review?(doug.turner)
Attachment #559800 - Flags: review?(doug.turner) → review+
Keywords: checkin-needed
Assignee: nobody → jitenmt
Keywords: checkin-needed
Whiteboard: [MemShrink:P2] → [MemShrink:P2][inbound]
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink:P2][inbound] → [MemShrink:P2]
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: