Closed Bug 999478 Opened 6 years ago Closed 6 years ago

Free the preloaded callscreen under memory pressure

Categories

(Firefox OS Graveyard :: Gaia::Dialer, defect)

x86
macOS
defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S1 (9may)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: etienne, Assigned: etienne)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Depends on: 990003
This would make bug 996002 useless. So if we do it we should close both bugs.

We currently keep the callscreen preloaded at all time for better performance, this costs us about 1MB of ram in exchange for near-instantaneous reaction time.
Attached patch Patch proposal (obsolete) — Splinter Review
Here's a patch doing just that.

Looks like it makes the call screen roughly 1s -> 1.5s slower to show up.
Interestingly enough it's pretty much the same time for a tarako and a keon.
Assignee: nobody → etienne
The patch is ready, so it's just a matter of taking a decision.
Vivien, Fabrice, you guys don't agree on this one :)

FWIW I think we could land this patch on 1.3t where the callscreen stills shows un in less than 2 seconds when receiving a call while watching a video.
But keep it as is on master in order to preserve the direct user feedback when you press the call button on higher end devices.
Flags: needinfo?(fabrice)
Flags: needinfo?(21)
(In reply to Etienne Segonzac (:etienne) from comment #3)
> But keep it as is on master in order to preserve the direct user feedback
> when you press the call button on higher end devices.

... while still working on bug 996002 obviously.
Duplicate of this bug: 996002
We really need to do that, but we also need to listen for the memory pressure in the system app to get rid of the frame when it has been opened but is not in use anymore.
Flags: needinfo?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #6)
> We really need to do that, but we also need to listen for the memory
> pressure in the system app to get rid of the frame when it has been opened
> but is not in use anymore.

Yes, memory pressure is the best of both world.
We won't waste 1MB on the tarako under pressure, but we'll keep the instant feedback on higher end phones.

Clearing Vivien's needinfo since I think we can safely move forward with this approach for all branches.
Flags: needinfo?(21)
1.3t+ because Fabrice says so.
blocking-b2g: --- → 1.3T?
Summary: Load the callscreen on demand instead of always having it loaded → Free the preloaded callscreen under memory pressure
Attached file Gaia PR
Attachment #8410310 - Attachment is obsolete: true
Attachment #8410956 - Flags: review?(21)
Fabrice, what about using memory-pressure-ongoing in order to try to favor keeping the call screen over various GC/apps etc ? (it can be easily expose to the system app with a 3 lines patch like: http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#723)

Or is it too late in general ? (in the future I would like more messages from the underlying code, like a message when the memory pressure is over, so we can reload the call screen. That may lead to some no-memory/free-memory cycles so we will need to be smart and wise about when to use that).
Flags: needinfo?(fabrice)
Comment on attachment 8410956 [details] [review]
Gaia PR

In general the code looks good. But I would like to see if we can't use the ongoing message instead. So let's wait for Fabrice to see if it is doable or not in practice (if i says 'yes', then it's a r-, if he says 'no' then you can carry forward my r+ with the small nit addressed.)
Attachment #8410956 - Flags: review?(21)
Let's land this patch and explore the "ongoing" part in a follow up.
Flags: needinfo?(fabrice)
Blocks: 1000248
Comment on attachment 8410956 [details] [review]
Gaia PR

Follow up filed, nit addressed, carrying the r+.
Attachment #8410956 - Flags: review+
(In reply to Vivien Nicolas (:vingtetun) (:21) (NOT reading bugmails, needinfo? please) from comment #10)
> Or is it too late in general ? (in the future I would like more messages
> from the underlying code, like a message when the memory pressure is over,
> so we can reload the call screen. That may lead to some
> no-memory/free-memory cycles so we will need to be smart and wise about when
> to use that).

We can expose such an event with ease though I wouldn't be too keen on using it for recreating the callscreen as it could create memory pressure cycles were we routinely kill and re-create it.

One alternative would be to re-create the callscreen on a timeout, that could have the same effect but could be tuned to avoid re-creating it too soon.

When we detect a low memory condition we re-check it every |gonk.systemMemoryPressureRecoveryPollMS| milliseconds (that's 5000 currently). If the pressure isn't over yet we send an ongoing event and wait again. So you could already re-create the callscreen on a timeout longer than |gonk.systemMemoryPressureRecoveryPollMS| and which would be reset every time you get another memory pressure event (ongoing or not). Making the timeout reasonably long (two times that value?) should prevent unwanted fluctuations.
Given the memory savings and :fabrice mentioned risk is manageable blocking on this,.
blocking-b2g: 1.3T? → 1.3T+
Hi! James,

Check in needed. Thanks

--
Keven
Flags: needinfo?(james.zhang)
Keywords: checkin-needed
Yang, please land it to v1.3t.
Flags: needinfo?(james.zhang) → needinfo?(yang.zhao)
Have confilicts when landing it to v1.3t.Please help to resovle it first.Thank you.
Flags: needinfo?(yang.zhao)
Flags: needinfo?(kkuo)
Flags: needinfo?(james.zhang)
Hi! Etienne,

Uplift conflict. Could you help? Thanks

--
Keven
Flags: needinfo?(kkuo) → needinfo?(etienne)
(In reply to Keven Kuo [:kkuo] from comment #19)
> Hi! Etienne,
> 
> Uplift conflict. Could you help? Thanks

I will uplift it on 1.3t once it lands on master.
Flags: needinfo?(etienne)
Master: https://github.com/mozilla-b2g/gaia/commit/03da6aab8a033d83835a245c968ce4dd2f06409e
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Flags: needinfo?(james.zhang)
Hi! Etienne,

Thanks for helping.

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