Closed
Bug 971728
Opened 7 years ago
Closed 7 years ago
Use memory-pressure events instead of minimization when sending an application into the background
Categories
(Firefox OS Graveyard :: General, defect, P2)
Tracking
(blocking-b2g:1.3T+, b2g-v1.3T fixed)
People
(Reporter: gsvelto, Assigned: gsvelto)
References
Details
(Keywords: perf, Whiteboard: [c=memory p= s= u=tarako])
Attachments
(1 file)
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).
Assignee | ||
Comment 1•7 years ago
|
||
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Comment 3•7 years ago
|
||
traige: will give this patch a try on tarako before deciding. Thanks Fabrice
Flags: needinfo?(fabrice)
Updated•7 years ago
|
Comment 4•7 years ago
|
||
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.
Flags: needinfo?(fabrice)
Assignee | ||
Comment 5•7 years ago
|
||
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
Attachment #8374849 -
Flags: review?(khuey)
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
Attachment #8374849 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 9•7 years ago
|
||
(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.
Keywords: checkin-needed
Comment 10•7 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/3d18224c5065
Keywords: checkin-needed
Comment 11•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3d18224c5065
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 13•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/2c30c27029a5
status-b2g-v1.3T:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•