Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Allow per-process GC/CC from about:memory

RESOLVED FIXED in mozilla9

Status

()

Toolkit
about:memory
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: njn, Assigned: deLta30)

Tracking

unspecified
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Depends on: 654041
(Reporter)

Updated

6 years ago
Assignee: nnethercote → nobody
Component: General → about:memory
QA Contact: general → about.memory
(Assignee)

Comment 1

6 years ago
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.
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
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?
(Reporter)

Comment 5

6 years ago
dougt, IIRC you implemented the multi-process support in about:memory -- how does comment 3 and comment 4 sound to you?
(Reporter)

Updated

6 years ago
Whiteboard: [MemShrink]

Comment 6

6 years ago
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?

Comment 7

6 years ago
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.
(Assignee)

Comment 8

6 years ago
Why isn't adding an IPDL message a good idea?

Comment 9

6 years ago
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.
(Reporter)

Updated

6 years ago
Whiteboard: [MemShrink] → [MemShrink:P2]
(Assignee)

Comment 11

6 years ago
Created attachment 559198 [details] [diff] [review]
Added messaging mechanism to send messages to content processes

review?
(Assignee)

Comment 12

6 years ago
Need test cases.
(Reporter)

Comment 13

6 years ago
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.
(Assignee)

Updated

6 years ago
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-
(Assignee)

Comment 15

6 years ago
Created attachment 559752 [details] [diff] [review]
Made changes according to suggestions
Attachment #559752 - Flags: review?(doug.turner)
(Assignee)

Comment 16

6 years ago
Created attachment 559759 [details] [diff] [review]
Sorry,slight changes were left.Review this one,not the last one.
Attachment #559752 - Attachment is obsolete: true
Attachment #559752 - Flags: review?(doug.turner)
Attachment #559759 - Flags: review?(doug.turner)

Updated

6 years ago
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
(Assignee)

Comment 18

6 years ago
Created attachment 559800 [details] [diff] [review]
Fixed whitespace

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)

Updated

6 years ago
Attachment #559800 - Flags: review?(doug.turner) → review+

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → jitenmt

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [MemShrink:P2] → [MemShrink:P2][inbound]
https://hg.mozilla.org/mozilla-central/rev/e3cfe5ae818c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink:P2][inbound] → [MemShrink:P2]
Target Milestone: --- → mozilla9
Oops, wrong URL.  I mean https://hg.mozilla.org/mozilla-central/rev/a1268291394f
You need to log in before you can comment on or make changes to this bug.