More considered handling of memory pressure notifications

RESOLVED FIXED in Firefox 39

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: donrhummy, Assigned: snorp, Mentored)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
Firefox 39
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed, fennec39+)

Details

(Whiteboard: [good next bug][lang=java][see comment 9])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0
Build ID: 2014012800

Steps to reproduce:

Loaded 6 tabs in FF Beta on a new Nexus 9 with Android 5.0.


Actual results:

It only keeps 1-2 tabs in memory at a time and has to reload the rest. (And leaving FF to go to another app and returning acts like a complete new reload and has to reload even the top tab)


Expected results:

It should have been able to handle keeping 6 tabs in memory. On an old 2012 1st Gen Nexus 7 and a Nexus 4 both, FF Beta is able to handle having 12+ tabs with 6-7 loaded at once and can switch between them without issue.

The Nexus 9 should be able to handle this as well, so there might be something FF Beta needs to do differently on newer devices? But if a Nexus 7 1st gen can handle more than the Nexus 9 in FF Beta, then something's wrong. (Chrome can handle as many tabs on the Nexus 9 as the Nexus 7, so it's not the device)

Updated

3 years ago
tracking-fennec: --- → ?
We need to pick up some Nexus 9 devices and do some memory profiling.
Assignee: nobody → rnewman
tracking-fennec: ? → +
Two avenues here:

* Memory pressure events are firing more frequently on L.
* Nexus 9 is a very high DPI device; are we using more memory on that device?

So let's investigate memory profiles on that device.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Linux → Android
Hardware: x86_64 → ARM
Summary: Firefox Beta is constantly having to reload tabs between switching on nexus 9 → Firefox Beta is constantly having to reload tabs between switching on Nexus 9
I have a Nexus 9, any specific instructions (or build) for verbose memory logging?

Reporter, also, is this reproducible on Firefox 35 (Beta)?
(Reporter)

Comment 4

3 years ago
(In reply to Aaron Train [:aaronmt] from comment #3)
> I have a Nexus 9, any specific instructions (or build) for verbose memory
> logging?
> 
> Reporter, also, is this reproducible on Firefox 35 (Beta)?

I am using Firefox Beta. (And yes, it happens in Beta)
I did some profiling.

As expected, we're allocating a lot of memory for thumbnails: between 20 and 250KB per thumbnail. This isn't insane, but it's more than we might want.

Of particular interest: BitmapFactory.decodeResourceStream (with a call stack starting in BrowserApp.startActionModeCompat) ends up allocating a non-movable array (dalvik.system.VMRuntime.newNonMovableArray). This will cause memory fragmentation. I'm not sure if this was the case on earlier versions of Android.

The same is true for TabStripItemView.updateFromTab.

Still looking at the rest.
Filed Bug 1116313 for the huge amount of work Fennec does when switching tabs, which is certainly noticeable on the N9.
As soon as you hit Home:

12-29 16:49:21.965 D/GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 80


public static final int TRIM_MEMORY_COMPLETE
Added in API level 14

Level for onTrimMemory(int): the process is nearing the end of the background LRU list, and if more memory isn't found soon it will be killed.
Constant Value: 80 (0x00000050)


This is with a ton of apps in the back stack, but none recently used.

So yes, we're getting onTrimMemory very aggressively.
Oh, and you get this when hitting the task switcher, too, even if Fennec is still the foreground app!

12-29 16:51:00.016 D/GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 80

art claims:

12-29 16:51:41.723 I/art     (  507): Background partial concurrent mark sweep GC freed 29099(1478KB) AllocSpace objects, 19(17MB) LOS objects, 30% free, 36MB/52MB, paused 677us total 148.345ms
Preliminary recommendations:

We need to get both our resting and active footprints down, stat.

We get moderate, low, then critical memory pressure warnings during normal use:

15:55:39 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 5
15:57:00 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 80
15:57:01 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 80
15:57:28 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 80
15:59:36 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 80
16:01:03 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 10
16:01:11 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 15


Once we get there, it's real easy to hit it again just by loading the pages that were swapped out:

16:04:36 GeckoMemoryMonitor(10872): Decreased memory pressure to 3
16:09:36 GeckoMemoryMonitor(10872): Decreased memory pressure to 2
16:14:36 GeckoMemoryMonitor(10872): Decreased memory pressure to 1
16:19:36 GeckoMemoryMonitor(10872): Decreased memory pressure to 0
16:33:56 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 10
16:38:56 GeckoMemoryMonitor(10872): Decreased memory pressure to 1
16:43:06 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 15
16:45:01 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 15
16:48:34 GeckoMemoryMonitor(10872): onTrimMemory() notification received with level 15


Additionally, this logic is wrong:


        if (level >= ComponentCallbacks2.TRIM_MEMORY_COMPLETE) {
            increaseMemoryPressure(MEMORY_PRESSURE_HIGH);
        } else if (level >= ComponentCallbacks2.TRIM_MEMORY_MODERATE) {
            increaseMemoryPressure(MEMORY_PRESSURE_MEDIUM);
        } else if (level >= ComponentCallbacks2.TRIM_MEMORY_UI_HIDDEN) {
            // includes TRIM_MEMORY_BACKGROUND
            increaseMemoryPressure(MEMORY_PRESSURE_CLEANUP);
        } else {
            // levels down here mean gecko is the foreground process so we
            // should be less aggressive with wiping memory as it may impact
            // user experience.
            increaseMemoryPressure(MEMORY_PRESSURE_LOW);
        }

COMPLETE is not really higher than MODERATE, and it's definitely not the next step up.

COMPLETE is sent when we're backgrounded. We're backgrounded when such fun things as tapping a notification or looking for a moment at the task switcher -- i.e., a lot. Throwing away user state in this case is the wrong thing to do -- we should throw away bitmaps, resources, DB handles, UI fragments, etc. instead.

We should be responding to TRIM_MEMORY_RUNNING_CRITICAL, not COMPLETE to go to MEMORY_PRESSURE_HIGH. When we get _COMPLETE we should probably dump everything *but* open pages!
Morphing this bug to something better scoped, but the real solution to this bug is to spend a significant investment of time in reducing our memory footprint: Bug 933814, and probably more improvements, too.

That means a focus on space and time efficiency for frontend features (cf Bug 1116313), on reducing the bloatedness of Gecko (even to the point of turning off and not shipping parts of the web platform), and otherwise aggressively profiling and cutting.
Depends on: 933814
Summary: Firefox Beta is constantly having to reload tabs between switching on Nexus 9 → More considered handling of memory pressure notifications
I filed a bunch of dependencies of Bug 933814, notably Bug 1116351.
Now that this has been scoped down, I think it's better suited for another contributor. I don't have the cycles for it, and if I did I'd spend them on Bug 1116351.
Assignee: rnewman → nobody
Mentor: rnewman
Whiteboard: [good second bug][lang=java]
Blocks: 1110554
Hardware: ARM → All
Whiteboard: [good second bug][lang=java] → [good second bug][lang=java][see comment 9]
Version: Firefox 34 → Trunk

Updated

3 years ago
Summary: More considered handling of memory pressure notifications → Consider improved handling of memory pressure notifications
That's not what that means.
Summary: Consider improved handling of memory pressure notifications → More considered handling of memory pressure notifications
Whiteboard: [good second bug][lang=java][see comment 9] → [good next bug][lang=java][see comment 9]
This is quite probably a cause for our user complaints about perf on Lollipop.
tracking-fennec: + → ?
Assignee: nobody → snorp
tracking-fennec: ? → 39+
Alright, so I've been playing around some to see what kind of onTrimMemory() notifications we get. On a Nexus 10 with Lollipop, I frequently get TRIM_MEMORY_COMPLETE just by opening the tab switcher or hitting the home button, as :rnewman says in comment #9. Sometimes it's the very first memory event for me, which just seems entirely wrong. I think we should probably just ignore that one, and instead respond properly to the other ones (which should precede COMPLETE in an actual memory pressure scenario).

Interestingly, I noticed we don't even do anything for LOW/UI_HIDDEN/BACKGROUND events. We should probably fix that too.
Created attachment 8580176 [details] [diff] [review]
Improve handling of Android memory pressure events
Attachment #8580176 - Flags: review?(rnewman)
(Reporter)

Comment 17

3 years ago
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #16)
> Created attachment 8580176 [details] [diff] [review]
> Improve handling of Android memory pressure events

When will this reach Firefox Beta (on Android) so I can help test?
Comment on attachment 8580176 [details] [diff] [review]
Improve handling of Android memory pressure events

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

OK, I think this does what you say it does.

::: mobile/android/base/MemoryMonitor.java
@@ +97,5 @@
> +        if (level == ComponentCallbacks2.TRIM_MEMORY_COMPLETE) {
> +            // We seem to get this just by entering the task switcher or hitting the home button.
> +            // Seems bogus, because we are the foreground app, or at least not at the end of the LRU list.
> +            // Just ignore it, and if there is a real memory pressure event (CRITICAL, MODERATE, etc),
> +            // we'll respond appropriately.

Pointer to this bug?

@@ +105,5 @@
> +        switch (level) {
> +            case ComponentCallbacks2.TRIM_MEMORY_RUNNING_CRITICAL:
> +            case ComponentCallbacks2.TRIM_MEMORY_MODERATE:
> +                // TRIM_MEMORY_MODERATE is the highest level we'll respond to while backgrounded
> +                increaseMemoryPressure(MEMORY_PRESSURE_HIGH);

This change to HIGH from MEDIUM for TRIM_MEMORY_MODERATE needs careful testing, I'd imagine.
Attachment #8580176 - Flags: review?(rnewman) → review+
(In reply to Richard Newman [:rnewman] from comment #18)
> 
> This change to HIGH from MEDIUM for TRIM_MEMORY_MODERATE needs careful
> testing, I'd imagine.

We really don't have separate levels within gecko, so it's really doing the same thing. MEDIUM/HIGH both just send a 'memory-pressure' gecko event.
https://hg.mozilla.org/mozilla-central/rev/9abdbcdced53
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
You need to log in before you can comment on or make changes to this bug.