Investigate tweaking bfache preferences on Android

RESOLVED FIXED in Firefox 62

Status

()

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: JanH, Assigned: JanH)

Tracking

unspecified
Firefox 62
All
Android
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(4 attachments)

Since we decided to turn on the bfcache in bug 472368, browser.sessionhistory.max_entries has always remained at 1, meaning we store at most one entry into the bfache.

We should investigate whether we could finally safely increase that value, although I can't say either whether the automatic algorithm (enabled by setting that pref to -1, see https://hg.mozilla.org/mozilla-central/annotate/71224049c0b52ab190564d3ea0eab089a159a4cf/docshell/shistory/nsSHistory.cpp#l263) might or might not still be too optimistic for us, although I guess we could always decide to tweak it with an #ifdef ANDROID.

Our current browser.sessionhistory.contentViewerTimeout will also mean that bfcache entries will expire sooner than on desktop and on a memory pressure event we drop the bfcache completely anyway, which should protect us a bit against excessive memory usage, but we should probably still test first how much of an impact increasing that pref could have.

Speaking of memory pressure, we then disable the bfache for the remainder of the session until the app is restarted (https://dxr.mozilla.org/mozilla-central/rev/71224049c0b52ab190564d3ea0eab089a159a4cf/mobile/android/chrome/content/MemoryObserver.js#41) - maybe we should cautiously try re-enabling it when our internal memory pressure state decays back to 0 (we currently don't have a notification for that, though).
Duplicate of this bug: 749321
Actually I've just realised that the value of 1 was set when Fennec was still running on the N810, which had a whopping 128 MB of memory.
Even on a hand-wavy scale with conservative assumptions that's long enough ago that we should be able to afford a somewhat larger value here.
If initially we still want to be absolutely sure not to overdo things, subtracting 15.9 instead of 14 at [1] will give
 256 MB -> 1
 512 MB -> 2
 768 MB -> 2
1024 MB -> 3
2048 MB -> 5
3072 MB -> 7
4096 MB -> 8

(In reply to Jan Henning [:JanH] from comment #0)
> Our current browser.sessionhistory.contentViewerTimeout will also mean that
> bfcache entries will expire sooner than on desktop
Currently after six minutes, to be precise.


[1] https://dxr.mozilla.org/mozilla-central/rev/de32269720d056972b85f4eec5f0a8286de6e3af/docshell/shistory/nsSHistory.cpp#299
Assignee: nobody → jh+bugzilla
Comment on attachment 8973511 [details]
Bug 1335148 - Part 1: Dynamically determine content viewer count on Android, too.

https://reviewboard.mozilla.org/r/241862/#review247960
Attachment #8973511 - Flags: review?(snorp) → review+
Comment on attachment 8973512 [details]
Bug 1335148 - Part 2: Introduce notification for end of memory pressure.

https://reviewboard.mozilla.org/r/241864/#review247962
Attachment #8973512 - Flags: review+
Comment on attachment 8973513 [details]
Bug 1335148 - Part 3: Notify Gecko and turn bfcache back on when Fennec memory pressure decays to 0.

https://reviewboard.mozilla.org/r/241872/#review247964
Attachment #8973513 - Flags: review?(snorp) → review+
Comment on attachment 8973514 [details]
Bug 1335148 - Part 4: Remove unused max_decoded_image_kb pref.

https://reviewboard.mozilla.org/r/241866/#review247966
Attachment #8973514 - Flags: review?(snorp) → review+
Attachment #8973512 - Flags: review?(n.nethercote) → review?(gsvelto)
Comment on attachment 8973512 [details]
Bug 1335148 - Part 2: Introduce notification for end of memory pressure.

https://reviewboard.mozilla.org/r/241864/#review248176

This is fine for now. I'm introducing a new way to monitor memory usage on desktop platforms in bug 1451005 which will make low-memory detection more robust and will allow me to gather more data on low-memory scenarios. So if we need to change the queuing behavior it's better to do it afterwards, when we'll have more usage data for these notifications.
Attachment #8973512 - Flags: review?(gsvelto) → review+
Comment on attachment 8973511 [details]
Bug 1335148 - Part 1: Dynamically determine content viewer count on Android, too.

https://reviewboard.mozilla.org/r/241862/#review248392

r=me

::: docshell/shistory/nsSHistory.cpp:46
(Diff revision 2)
>  #define CONTENT_VIEWER_TIMEOUT_SECONDS "browser.sessionhistory.contentViewerTimeout"
>  
>  // Default this to time out unused content viewers after 30 minutes
>  #define CONTENT_VIEWER_TIMEOUT_SECONDS_DEFAULT (30 * 60)
>  
> +#ifdef ANDROID

This really needs to be somewhere near the comment that maps RAM amounts to numbers of viewers.  Otherwise it's really not clear where these magic numbers are coming from.
Attachment #8973511 - Flags: review?(bzbarsky) → review+
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/d8e7c56a2a85
Part 1: Dynamically determine content viewer count on Android, too. r=bz,snorp
https://hg.mozilla.org/integration/autoland/rev/6e79931930ee
Part 2: Introduce notification for end of memory pressure. r=gsvelto,snorp
https://hg.mozilla.org/integration/autoland/rev/36e95b6ba7a6
Part 3: Notify Gecko and turn bfcache back on when Fennec memory pressure decays to 0. r=snorp
https://hg.mozilla.org/integration/autoland/rev/e5332f31998b
Part 4: Remove unused max_decoded_image_kb pref. r=snorp
You need to log in before you can comment on or make changes to this bug.