Closed Bug 699279 Opened 8 years ago Closed 8 years ago
On memory pressure, JS should do a SHRINK GC
A shrink GC attempts to get rid of things more aggressively than a normal GC, so maybe we should do that in nsMemoryPressureObserver::Observe instead of a normal one. One goal of this is to get about:memory to dump more memory when the buttons are clicked. I'm not really sure of the right component for this. I think this will require exposing an additional JS API kind of thing, then modifying XPConnect, nsJSEnvironment and nsMemoryPressureObserver to also add in this additional hook, assuming the current convoluted path the call follows is the right way.
Another goal is to get Fennec to dump as much memory as possible when it's sent into the background. (Ironically enough, we currently have code not to trigger a GC on memory pressure when we're Fennec, because Fennec does its own memory pressure watching. But once the plumbing is there, we can get Fennec to do a shrink gc.)
Terrence, can you take this? It shouldn't be hard, just a bit of plumbing through xpconnect, as per comment 0.
Assignee: general → terrence
Whiteboard: [MemShrink] → [MemShrink:P2]
Yeah, with the existing pointers, I should be able to take this.
The call stack between the observer and the GC appears to be something like. -- in dom/base/nsJSEnvironment.cpp 1) nsMemoryPressureObserver::Observe 2) nsJSContext::GarbageCollectNow 3) nsContentUtils::XPConnect()->GarbageCollect -- in js/xpconnect/src/nsXPConnect.cpp 4) nsXPConnect::GarbageCollect 5) nsXPConnect::Collect -- in jsapi.cpp 6) JS_GC 7) JS_CompartmentGC -- in jsgc.cpp 8) js_GC 9) ... 10) profit All of these files appear to have access to jsgc.h (and jsprvtd.h too for that matter), so a lazier me could just use JSGCInvocationKind in all of these signatures right now and be done with it. I think what we probably want to do is either move JSGCInvocationKind to a public header or create a new public invocation kind, if the internal one has fields that shouldn't be exposed to the full browser. What do you think?
Status: NEW → ASSIGNED
Sounds reasonable to me.
The bottleneck here is JS_GC. We probably don't want to add a new parameter because it would break a lot of callers in Firefox and elsewhere. We can't add default parameters because jsapi.h is C-only. I would suggest adding a JS_ShrinkingGC function to jsfriendapi. Then you could add an extra bool parameter to the other ones above it.
This is less hideous than I thought it would be.
Attachment #573388 - Flags: review?(wmccloskey)
Ah, yes, that's not too bad. You need to change the uuid thing when you change an idl. Seems a little weird to add a parameter to nsCycleCollectionJSRuntime we never really use, but I suppose the alternative is splitting off a new Collect interface in XPConnect, depending on whether you want to shrink or not which is probably gross.
(In reply to Andrew McCreight [:mccr8] from comment #10) > Ah, yes, that's not too bad. You need to change the uuid thing when you > change an idl. Seems a little weird to add a parameter to > nsCycleCollectionJSRuntime we never really use, but I suppose the > alternative is splitting off a new Collect interface in XPConnect, depending > on whether you want to shrink or not which is probably gross. Thanks for the feedback! I will update the uuid and start a try run when try is online again.
Comment on attachment 573388 [details] [diff] [review] v1: appears to work as intended. Looking this over, I'm probably not the best person to review. Do we need to worry about add-on compatibility since we're changing IDL?
Attachment #573388 - Flags: review?(wmccloskey) → review?(mrbkap)
It seems unlikely that any addons manually call the GC, but I threw a flag on there anyways.
Attachment #573388 - Flags: review?(mrbkap) → review+
Thanks for the review!
Attachment #573388 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.