Last Comment Bug 803575 - [Native Fennec] Expire tabs based on LRU and age
: [Native Fennec] Expire tabs based on LRU and age
Status: RESOLVED FIXED
[MemShrink]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: -- normal (vote)
: Firefox 19
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
: general
Mentors:
Depends on: 792143 801818
Blocks: 256meg 816381
  Show dependency treegraph
 
Reported: 2012-10-19 08:51 PDT by Kartikaya Gupta (email:kats@mozilla.com)
Modified: 2012-11-29 08:47 PST (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (4.02 KB, patch)
2012-10-24 14:19 PDT, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details | Diff | Splinter Review
Patch (v2) (5.68 KB, patch)
2012-10-31 11:30 PDT, Kartikaya Gupta (email:kats@mozilla.com)
mark.finkle: review+
Details | Diff | Splinter Review

Description Kartikaya Gupta (email:kats@mozilla.com) 2012-10-19 08:51:56 PDT
We need to figure out if/when to turn on the tab expiration added in bug 792143. Some possible options:

1) Pref it on for all ARMv6 builds at compile-time
2) Pref it on for low-memory devices at run-time (assuming bug 801818 is fixed)
3) Wait until we hit a low-memory situation at runtime and then turn it on
4) Some combination of the above
5) Other?
Comment 1 Brad Lassey [:blassey] (use needinfo?) 2012-10-22 15:49:42 PDT
my preference would be for #3. I think taras would prefer having it on all the time (for battery consumption rather than memory), CC'ing him.
Comment 2 (dormant account) 2012-10-22 16:09:34 PDT
(In reply to Brad Lassey [:blassey] from comment #1)
> my preference would be for #3. I think taras would prefer having it on all
> the time (for battery consumption rather than memory), CC'ing him.

I think #3 is a good start, can investigate more aggressive approaches once #3 lands.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2012-10-22 16:42:25 PDT
Agreed that #3 is a good start. It would be nice if we could make this a session-only pref. I mean, each time we'd start a new Firefox, we'd start _not_ expiring tabs. We could do that by setting the "default branch" version of the preference:

let defaults = Services.prefs.getDefaultBranch();
defaults.setIntPref(...);

Such a pref value is only retained while Firefox is running and is reset to the real default value upon restart.

Food for thought.
Comment 4 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-23 10:34:26 PDT
Was discussing this with Brad today, and I want to clarify the existing behaviour and exactly what this bug is for.

Currently:
1) we unconditionally zombify all background tabs when we get a memory-pressure event (this was added in bug 784040)
2) time-based tab zombification (added in bug 792143) is completely disabled

"Time-based tab zombification", if enabled, will trigger zombification of a zero or one tabs when a new tab is opened. The tab that is zombified is the least-recently-used tab, if and only if the tab hasn't been used for browser.tabs.zombieTime seconds.

This bug is about when to enable "time-based tab zombification". My preference is to turn it on by default for low-memory devices, and to turn it on after receiving the first memory-pressure event on high-memory devices. I can add another boolean check or pref control exactly when the behaviour is enabled, if needed (or I could try to do it using the "default branch" of the pref as mark suggested).
Comment 5 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-24 14:19:30 PDT
Created attachment 674830 [details] [diff] [review]
Patch

This patch (1) assumes that my patch from bug 801818 is in the tree and (2) calls the new behaviour "pre-emptive zombies" because i suck at coming up with good names for things. Suggestions welcome, but I'll wait on requesting review until bug 801818 is actually landed.
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2012-10-24 21:11:46 PDT
You could refer to this action as "tab expiration" (you filed it that way) and use:
Tabs._enablePreemptiveZombies -> Tabs._enableExpiration
browser.tabs.zombieTime -> browser.tabs.expirationTime

I'm fine with zombies too, so up to you :)
Comment 7 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-31 11:30:41 PDT
Created attachment 677088 [details] [diff] [review]
Patch (v2)
Comment 8 Mark Finkle (:mfinkle) (use needinfo?) 2012-11-01 14:22:41 PDT
Comment on attachment 677088 [details] [diff] [review]
Patch (v2)

Looks like a good start. Would telemetry probes for expiring tabs be useful to help us see how often the code is executing?
Comment 9 Kartikaya Gupta (email:kats@mozilla.com) 2012-11-02 08:02:16 PDT
(In reply to Mark Finkle (:mfinkle) from comment #8)
> Looks like a good start. Would telemetry probes for expiring tabs be useful
> to help us see how often the code is executing?

Good call, I've filed bug 808003 as a follow-up for that.
Comment 10 Kartikaya Gupta (email:kats@mozilla.com) 2012-11-02 08:03:53 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5981344041a1
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-11-02 17:23:30 PDT
https://hg.mozilla.org/mozilla-central/rev/5981344041a1

Note You need to log in before you can comment on or make changes to this bug.