Closed
Bug 1376137
Opened 8 years ago
Closed 8 years ago
MemoryMonitor has Android memory pressure levels the wrong way around
Categories
(Firefox for Android Graveyard :: General, enhancement)
Tracking
(firefox55 fixed, firefox56 fixed)
RESOLVED
FIXED
Firefox 56
People
(Reporter: JanH, Assigned: JanH)
Details
Attachments
(1 file)
|
59 bytes,
text/x-review-board-request
|
snorp
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
According to the Android documentation (https://developer.android.com/reference/android/content/ComponentCallbacks2.html), the initial warning when we're in foreground is TRIM_MEMORY_RUNNING_MODERATE, which is then followed by TRIM_MEMORY_RUNNING_LOW and finally TRIM_MEMORY_RUNNING_CRITICAL.
Our current logic (https://dxr.mozilla.org/mozilla-central/rev/92dc60b522d81862e52bff5cdb1b698eb5608658/mobile/android/base/java/org/mozilla/gecko/MemoryMonitor.java#120) however has the relationship to our own pressure levels swapped around - TRIM_MEMORY_RUNNING_MODERATE triggers MEMORY_PRESSURE_MEDIUM, while TRIM_MEMORY_RUNNING_LOW only triggers MEMORY_PRESSURE_LOW.
This might also explain why some users have complained that we unload tabs too early, because Gecko-side memory pressure is dispatched for MEMORY_PRESSURE_MEDIUM and higher.
| Comment hidden (mozreview-request) |
Comment 2•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8881965 [details]
Bug 1376137 - Fix Android memory pressure mapping.
https://reviewboard.mozilla.org/r/153032/#review158232
I vaguely remember some weird reason it was done this way, but I'm not finding it immediately, so I think this is change is fine.
Attachment #8881965 -
Flags: review?(snorp) → review+
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/0afe480c1642
Fix Android memory pressure mapping. r=snorp
Comment 4•8 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment 5•8 years ago
|
||
I think Bug 1106827 might be the context you're looking for, snorp.
| Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8881965 [details]
Bug 1376137 - Fix Android memory pressure mapping.
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1106827
[User impact if declined]: We start unloading tabs at the slightest hint of memory pressure from the OS instead of waiting for the following OS memory pressure level (where it then actually makes sense to start reducing our memory usage).
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just changed the mapping between OS and internal memory pressure levels to have the correct ordering according to the Android documentation.
[String changes made/needed]: none
Attachment #8881965 -
Flags: approval-mozilla-beta?
Comment 7•8 years ago
|
||
Comment on attachment 8881965 [details]
Bug 1376137 - Fix Android memory pressure mapping.
fennec fix for memory pressure handling, beta55+
Attachment #8881965 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 8•8 years ago
|
||
| bugherder uplift | ||
status-firefox55:
--- → fixed
Updated•5 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•