Closed Bug 744519 Opened 9 years ago Closed 7 years ago

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

Categories

(Core :: Widget: Android, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: bjacob, Assigned: blassey)

Details

Attachments

(1 file)

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.
> 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...
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.
(In reply to Gabriele Svelto [:gsvelto] from comment #2)
Is NS_DispatchMemoryPressure() safe to call from any thread?
(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
Assignee: nobody → blassey.bugs
Attachment #8339227 - Flags: review?(gsvelto)
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.
Attachment #8339227 - Flags: review?(gsvelto) → review+
https://hg.mozilla.org/mozilla-central/rev/1654d2d361bc
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.