971728 - Use memory-pressure events instead of minimization when sending an application into the background

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P2)

Description

[Gabriele Svelto [:gsvelto]]

While reading the issue in bug 961348 it occurred to me that whenever we launch a new application another one gets sent into the background (typically the homescreen); this triggers a minimization of the newly backgrounded application in order to save memory. See the code here:

http://hg.mozilla.org/mozilla-central/file/879038dcacb7/dom/ipc/ProcessPriorityManager.cpp#l1037

This is problematic: memory minimization is a very expensive procedure which issues three memory-pressure events and thus runs the GC/CC three times over. This procedure is long enough that it can interfere with the starting application and it also touches all the heap memory thrice making it detrimental for something like zRAM. In addition to this it could introduce glitches during fast app switching which forced us to introduce a procedure to cancel it in flight in bug 814771 which added a fair bit of complexity to the process.

Today I experimented with changing this logic to send a single memory-pressure event to the backgrounded process instead of a full minimize. I've then measured the resulting effect this had on memory consumption in a few tests (all done with heavy reference workloads). For every test I measured how much memory was saved by the memory minimization procedure and how much by a simple memory-pressure event. The tests were the following:

1) I've opened the dialer, went into the call log, went into contacts list, waited for it to settle then sent it into the background.

2) I've opened the SMS app, went into the big SMS thread, waited for it to settle then sent it into the background.

3) I've opened the Calendar app, swiped three times back then created an event, then sent it into the background.

These are the results I got which were fairly consistent:

+------+--------------+-----------------+
| test | minimization | memory-pressure |
+------+--------------+-----------------+
|    1 |      4.2 MiB |         4.1 MiB |
|    2 |      1.1 MiB |         1.1 MiB |
|    3 |      0.6 MiB |         0.6 MiB |
+------+--------------+-----------------+

I can do more testing but it seems to me that there's really no point in using a minimization over a memory-pressure event. From a CPU usage perspective minimization takes a fair bit of extra CPU usage for very little or no benefit.

Here's two profiles of the SMS app going into the background, first with minimization, >120ms are spent in the GC (look for the last CPU spike to the right):

http://people.mozilla.org/~bgirard/cleopatra/#report=9bc3e816fc0162c61bb1e25ae791b0a3efc74510

Then with a memory-pressure event, only ~50ms are spent in the GC:

http://people.mozilla.org/~bgirard/cleopatra/#report=de1173d7986d8c2fac58fa9df0ce64830200fdb0

I'm cooking up a patch to change this and I suggest testing this on a Tarako device to see if it has a positive impact on application startup (I don't have one, otherwise I would do it myself).

Comment 1

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH] Use memory-pressure events instead of minimization when sending an app to the background

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Nice!  Let's get this into triage today.

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

traige: will give this patch a try on tarako before deciding. Thanks Fabrice

Comment 4

[[:fabrice] Fabrice Desré]

I tested this patch on top of the current 1.3t branch. That seems to help a little with startup, but it's clearly not a huge gain. That's low risk though so I'm fine with taking it.

Comment 5

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8374849 [details] [diff] [review]
[PATCH] Use memory-pressure events instead of minimization when sending an app to the background

(In reply to Fabrice Desré [:fabrice] from comment #4)
> I tested this patch on top of the current 1.3t branch. That seems to help a
> little with startup, but it's clearly not a huge gain. That's low risk
> though so I'm fine with taking it.

Excellent, let's go then. This is a green try run with the patch applied:

https://tbpl.mozilla.org/?tree=Try&rev=7cf7656c6dcd

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8374849 [details] [diff] [review]
[PATCH] Use memory-pressure events instead of minimization when sending an app to the background

Review of attachment 8374849 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 7

[Thinker Li [:sinker]]

Does this patch conflict with bug 963477?

Comment 8

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

I don't think so.

Comment 9

[Gabriele Svelto [:gsvelto]]

(In reply to Thinker Li [:sinker] from comment #7)
> Does this patch conflict with bug 963477?

Definitely not, in fact it should mitigate the issue that caused bug 963477. Without this patch we were running three GC/CC cycles in a row when sending an app to the background, with this patch applied it goes down to only on GC/CC cycle. I'd suggest redoing the tests in bug 963477 with this patch applied and GC re-enabled to see how this affects startup time and memory consumption.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/3d18224c5065

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3d18224c5065

Comment 12

[Joe Cheng [:jcheng] (please needinfo)]

traige: 1.3T+ to get this into tarako

Comment 13

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/2c30c27029a5