Closed Bug 962963 Opened 9 years ago Closed 9 years ago

Be more aggressive about expiring tabs

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(3 files)

As seen in https://areweslimyet.com/mobile, tabs can hold onto a good bit of memory. We have mechanisms to help reduce the active tab count and reclaim some memory. One way is through tab expiration:

Expire (zombify) tabs after a given time period (3600 seconds)
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#8294

Looking at recent telemetry, tab expiration typically happens way longer than 3600 secs. The 25th percentile is over 4500 secs. 75th percentile is over 26000 secs. This is probably due to the way we fire the expiration code, which only happens when adding a new tab.

We could lower the expiration time and we could use a timer to trigger expiration.
I was thinking about whether we can do this more efficiently, in response to memory measurements.

For historical context, see Bug 744515 Comment 4, and Bug 744515 Comment 9, in which kats suggests using a SoftReference as a memory pressure detector.

For modern situational knowledge, read <http://developer.android.com/training/articles/memory.html#ReleaseMemoryAsTight>. Most of the contents of that document are ICS-onward, which is increasingly less of a problem.

I contend that we should be discarding tabs based on onTrimMemory calls; see MemoryMonitor.java. We already drop the FaviconCache, and (as far as I can tell) we invoke nsMemoryPressure's NS_DispatchMemoryPressure function.

The right thing to do here is probably to:

* Hook Java-side stuff into MemoryMonitor.
* Attach Gecko-side stuff to the MemPressure stuff emitted by nsMemoryPressure.
(In reply to Richard Newman [:rnewman] from comment #1)

> I contend that we should be discarding tabs based on onTrimMemory calls; see
> MemoryMonitor.java. We already drop the FaviconCache, and (as far as I can
> tell) we invoke nsMemoryPressure's NS_DispatchMemoryPressure function.

Sounds like you want #2 (or #4) from bug 962967
I filed the bugs separately because I felt:
* Tab expiration is "you haven't used this tab in a while, let zombify it"
* Tab discarding is "low memory condition: damn the tabs!"
This patch uses "heap-minimize" to perform expiration, if enabled. It also moves the trigger for expiration-on-newtab into the Tabs class since we have a handler now. I like that for decoupling code.
Assignee: nobody → mark.finkle
Attachment #8364411 - Flags: review?(margaret.leibovic)
Attachment #8364411 - Flags: review?(bugmail.mozilla)
This patch decouples more. The Tab.lastTouchedAt data is a property of the Tab, but Tabs was manipulating it. Since we always update it when we change the Tab.setActive, I just moved the update into setActive.
Attachment #8364413 - Flags: review?(margaret.leibovic)
Based on the long expiration times on Telemetry and the hunch that 3600 secs seems long for mobile usage patterns, this patch reduces the expiration time from 3600 secs (1 hr) to 900 secs (15 mins).

We can watch Telemetry for movement and listen for feedback about zombie tabs being a problem. Keeping in mind that we won't even try to expire tabs unless memory is an issue.
Attachment #8364415 - Flags: review?(bugmail.mozilla)
If we want to be more aggressive, we could make a timer in Tabs with "browser.tabs.expireTime" as the interval and use it as a trigger for expiring tabs.
Comment on attachment 8364411 [details] [diff] [review]
Trigger expiration on heap-minimize v0.1

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

::: mobile/android/chrome/content/browser.js
@@ +8285,5 @@
>            this._enableTabExpiration = true;
>            Services.obs.removeObserver(this, "memory-pressure");
> +        } else {
> +          // Use "heap-minimize" as a trigger to expire stale tabs
> +          this.expireLruTab();

Note that expireLruTab expires at most one tab at a time. So the comment here should say "expire the most stale tab, if there is one". Or you could change the behaviour of expireLruTab if you want.
Attachment #8364411 - Flags: review?(bugmail.mozilla) → review+
Attachment #8364415 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8364411 [details] [diff] [review]
Trigger expiration on heap-minimize v0.1

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

My one concern with this is being so aggressive about expiring tabs that we degrade the tabbed browsing experience for users. However, I see in the expireLruTab implementation that we only expire a tab if it hasn't been touched for more that 3600 seconds, so this means that a user opening a few tabs to research something doesn't need to worry about them expiring while flipping back and forth in one session. So I'm okay with this.
Attachment #8364411 - Flags: review?(margaret.leibovic) → review+
Attachment #8364413 - Flags: review?(margaret.leibovic) → review+
(In reply to :Margaret Leibovic from comment #9)
> Comment on attachment 8364411 [details] [diff] [review]
> Trigger expiration on heap-minimize v0.1
> 
> Review of attachment 8364411 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> My one concern with this is being so aggressive about expiring tabs that we
> degrade the tabbed browsing experience for users. However, I see in the
> expireLruTab implementation that we only expire a tab if it hasn't been
> touched for more that 3600 seconds, so this means that a user opening a few
> tabs to research something doesn't need to worry about them expiring while
> flipping back and forth in one session. So I'm okay with this.

The good news is: If you don't have a low-memory device and you don't hit an Android "low-memory" event, you'll never have a tab expire. This only kicks in for known low-memory situations.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.