744519 - Implement memory-pressure flag to react quicker to memory pressure, without waiting for event loop

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Out current mechanism to react to memory pressure events on Android goes through the following hoops:

 1. android calls onLowMemory, see GeckoApp.java
 2. this calls GeckoAppShell.onLowMemory(), see Java_org_mozilla_gecko_GeckoAppShell_onLowMemory in AndroidJNI.cpp
 3. this calls gAppShell->NotifyObservers with "memory-pressure" topic
 4. the gecko event loop delivers this event to observers (as in nsIObserver).

We need to react as quickly as possible, but the final step 4., the Gecko event delivery, can take more than 1 second in certain conditions.

A way to cut that delay would be if, in addition to calling NotifyObservers, step 3 also set a flag that Gecko code could read at any time. This would allow concerned Gecko code to react immediately. Typically, I would want to check this flag in WebGL functions, to be able to lose WebGL contexts quicker on memory pressure.

Comment 1

[Justin Lebar (not reading bugmail)]

> We need to react as quickly as possible, but the final step 4., the Gecko event delivery, can take 
> more than 1 second in certain conditions.

Have you looked at how the Windows low-memory detector (AvailableMemoryTracker.cpp) fires a memory-pressure event (ScheduleMemoryPressureEvent, implemented in nsThread.cpp)?

The event this triggers happens immediately after the last event finishes, rather than waiting its turn in the event loop.  This may be fast enough for you.

If it's not, we could definitely expose a function to read the sMemoryPressurePending flag...

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

I stumbled upon this bug while looking at FxOS memory pressure bugs and it seems that this should be pretty easy to fix since we've landed bug 876029. We have a fast-path to send memory pressure events using NS_DispatchMemoryPressure() so we just need to make the method visible to our Java code and then use it instead of dispatching an event here:

http://hg.mozilla.org/mozilla-central/file/77b5c6edfe96/mobile/android/base/MemoryMonitor.java#l155

I have never built Firefox for Android yet otherwise I'll give it a stab myself as it seems a pretty easy fix.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Gabriele Svelto [:gsvelto] from comment #2)
Is NS_DispatchMemoryPressure() safe to call from any thread?

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #3)
> Is NS_DispatchMemoryPressure() safe to call from any thread?

Yes, it just sets a global flag atomically and then schedules an event to wake up the main thread in case the event queue was empty:

http://hg.mozilla.org/mozilla-central/file/6ecf0c4dfcbe/xpcom/threads/nsMemoryPressure.cpp#l48

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: dispatchMemoryPressure.patch

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8339227 [details] [diff] [review]
dispatchMemoryPressure.patch

Review of attachment 8339227 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the small change I've described below.

::: mobile/android/base/MemoryMonitor.java
@@ +156,1 @@
>                  GeckoAppShell.sendEventToGecko(GeckoEvent.createLowMemoryEvent(level));

Remove this line as it is not necessary anymore. In fact leaving it would be detrimental as we would signal the memory-pressure condition twice, once with NS_DispatchMemoryPressure() and once with the event.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1654d2d361bc