999478 - Free the preloaded callscreen under memory pressure

Status: RESOLVED FIXED

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

Comment 1

[Etienne Segonzac (:etienne)]

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.

Comment 2

[Etienne Segonzac (:etienne)]

Attachment: Patch proposal

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.

Comment 3

[Etienne Segonzac (: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.

Comment 4

[Etienne Segonzac (:etienne)]

(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.

Comment 6

[[:fabrice] Fabrice Desré]

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.

Comment 7

[Etienne Segonzac (:etienne)]

(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.

Comment 8

[Etienne Segonzac (:etienne)]

1.3t+ because Fabrice says so.

Comment 9

[Etienne Segonzac (:etienne)]

Attachment: Gaia PR

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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).

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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.)

Comment 12

[[:fabrice] Fabrice Desré]

Let's land this patch and explore the "ongoing" part in a follow up.

Comment 13

[Etienne Segonzac (:etienne)]

Comment on attachment 8410956 [details] [review]
Gaia PR

Follow up filed, nit addressed, carrying the r+.

Comment 14

[Gabriele Svelto [:gsvelto]]

(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.

Comment 15

[bhavana bajaj [:bajaj]]

Given the memory savings and :fabrice mentioned risk is manageable blocking on this,.

Comment 16

[Keven Kuo [:kkuo]]

Hi! James,

Check in needed. Thanks

--
Keven

Comment 17

[James Zhang (Spreadtrum)]

Yang, please land it to v1.3t.

Comment 18

[yang.zhao]

Have confilicts when landing it to v1.3t.Please help to resovle it first.Thank you.

Comment 19

[Keven Kuo [:kkuo]]

Hi! Etienne,

Uplift conflict. Could you help? Thanks

--
Keven

Comment 20

[Etienne Segonzac (: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.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/03da6aab8a033d83835a245c968ce4dd2f06409e

Comment 22

[Etienne Segonzac (:etienne)]

Uplifted to 1.3t : https://github.com/mozilla-b2g/gaia/commit/f348044cf8beaa35cf82818d47f0a65c7a0f7c7a

Comment 23

[Keven Kuo [:kkuo]]

Hi! Etienne,

Thanks for helping.

--
Keven

Comment 24

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/38250fcbca0e3ae1ced0b4449a9c31a3d428d65e