Use interrupt-check to trigger full GCs on mem-pressure events.

NEW
Unassigned

Status

()

defect
6 years ago
3 years ago

People

(Reporter: nbp, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment)

As Far As I Understand, currently what is happening is the following:

 a/0. < in the mean time, the main thread is running some JavaScript code >

 b/0. /sys/kernel/mm/lowmemkiller/notify_trigger_active is modified
 b/1. MemoryPressureWatcher::Run() wake up and calls the Dispatch function[1]
 b/2. The dispatch function add an event to warn the main thread.

 a/1. The main thread complete the execution of the JavaScript code, process the event [2] and notify all observers [3].
 a/2. The JSEnv Observer[4] catch the message and trigger a full GC.

The problem with this scheme is that JavaScript might commit a lot of memory before giving back the hand to the event loop, which will cause the process to be killed by the kernel before giving a chance to shrink the heap size used by JS.

Ideally, we should interrupt the JavaScript execution to run the GC which is "requested" by the low-mem killer.  To do that, the dispatch function should have another mechanism to wake up the "JS Watchdog" [5] thread.

The callback mechanism seem to already have some instrumentation for the incremental GC without replacing the one-entry callback kept on a JSRuntime.  We should probably reuse this mechanism by calling a function similar to TriggerGC from the MemoryPressureWatcher thread and wake-up the watchdog thread such as the next time the interrupt-check is verified, the callback path [6] will do the shrinking GC.

[1] https://mxr.mozilla.org/mozilla-central/source/widget/gonk/GonkMemoryPressureMonitoring.cpp#51
[2] ???
[3] MemoryPressureRunnable::Run()
[4] nsJSEnvironmentObserver::Observe
[5] XPCJSRuntime::WatchdogMain
[6] js_InvokeOperationCallback
Summary: Use operation callback to trigger full GCs on mem-pressure events. → Use interrupt-check to trigger full GCs on mem-pressure events.
I am was looking if it was feasible to use the nsXPConnect::GetXPConnect function to trigger an operation call back with, but as of Bug 716167, this function asserts that it is only used in the main thread or in the CC thread.

bholley, do you think it would be fine to add another special case for the memory pressure watcher thread?
(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> bholley, do you think it would be fine to add another special case for the
> memory pressure watcher thread?

Not unless the memory pressure watcher thread has similar semantics to the CC thread (in that it's not really a thread at all, and the main thread is always fully blocked whenever it runs).
Yeah, the main thread halts, the CC thread starts up, then the CC thread does some kind of callback indicating that the JS runtime thread is now the CC thread, then runs, then halts, then the main thread starts up and does another callback to indicate that the main thread is now the JS runtime thread.
Ok, as I did not manage to get a working Otoro, I hook the memory pressure watcher to the volume button on a unagi build.

So far, I am able to trigger a memory pressure GC on all applications right away for the main runtime.  This patch still lacks Worker integration, but it should not be hard to add it.

Sadly, I am not convinced by my current approach, because including widget/gonk/GonkMemoryPressureMonitoring.h in xpcprivate.h implies the modifications of multiple (10) makefiles to make this header visible.

jlebar: I have no idea, where to make these changes visible in Gecko, to prevent adding that many includes.  Any recommendations?
Attachment #751812 - Flags: feedback?(justin.lebar+bug)
Sorry for letting this sit for so long.  I'm (hopefully) done with tef, so I
should be able to turn around new versions of this more quickly.

I can't speak to the JS bits here, but that doesn't seem to be what's giving
you difficulty.

> namespace mozilla {
> void InitGonkMemoryPressureMonitoring();
>-}
>+
>+namespace MemoryPressure {
>+
>+class FastObserver

The "Gonk" part of GonkMemoryPressureMonitoring refers to the Android-like (or,
Android-lite) operating system that B2G runs on.

This class doesn't have anything to do with Gonk.  Indeed, we could use it to
back the memory-pressure monitoring we have on Windows and in nsThread (see
xpcom/base/AvailableMemoryTracker.cpp and search for "memorypressure" in
nsThread.cpp).

I'm not sure we need a new observer class; I think you can probably get away
with mozilla::Observer (xpcom/glue/Observer.h) and mozilla::ObserverList,
mozilla::LinkedList, or nsTArray.

You'd still need some service which can own this ObserverList; you could put
that in a new file in xpcom/base or something.

Does that help?
(In reply to Justin Lebar [:jlebar] from comment #5)
> > namespace mozilla {
> > void InitGonkMemoryPressureMonitoring();
> >-}
> >+
> >+namespace MemoryPressure {
> >+
> >+class FastObserver
> 
> The "Gonk" part of GonkMemoryPressureMonitoring refers to the Android-like
> (or,
> Android-lite) operating system that B2G runs on.
> 
> This class doesn't have anything to do with Gonk.  Indeed, we could use it to
> back the memory-pressure monitoring we have on Windows and in nsThread (see
> xpcom/base/AvailableMemoryTracker.cpp and search for "memorypressure" in
> nsThread.cpp).
> 
> I'm not sure we need a new observer class; I think you can probably get away
> with mozilla::Observer (xpcom/glue/Observer.h) and mozilla::ObserverList,
> mozilla::LinkedList, or nsTArray.
> 
> You'd still need some service which can own this ObserverList; you could put
> that in a new file in xpcom/base or something.
> 
> Does that help?

Yes, a lot :)

I feel that xpcom/base/AvailableMemoryTracker.cpp should mostly be in widget/windows.

So I guess the new xpcom/base file (or nsThread) should own the memory pressure observer list and that GonkMemoryPressure will call ScheduleMemoryPressureEvent() to dispatch a memory pressure instead of doing the current signal.

Then I should modify the ScheduleMemoryPressureEvent() function to do the fast dispatch of the memory pressure event.  And as this is part of nsThread, I won't have any trouble to add it the main XPCJSRuntime and to do the same thing for the workers.

It took me a while to get the pipeline of events, but I feel that I understand it now,
Thanks :)
> I feel that xpcom/base/AvailableMemoryTracker.cpp should mostly be in widget/windows.

Probably!

> And as this is part of nsThread [...]

If we're modifying ScheduleMemoryPressureEvent and making it more generic, we should probably move this whole business into a new file, instead of piggybacking on nsThread.
Attachment #751812 - Flags: feedback?(justin.lebar+bug)
Depends on: 876029
Whiteboard: [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee: nicolas.b.pierron → nobody
Status: ASSIGNED → NEW
OS: Gonk (Firefox OS) → Windows
Hardware: ARM → All
You need to log in before you can comment on or make changes to this bug.