1715705 - Don't run multiple MEMORY_PRESSURE GCs concurrently in different processes

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, enhancement, P1)

Description

[Paul Bone [:pbone]]

Bug 1629064 implemented a system of scheduling GCs in different processes so that multiple processes wouldn't all try to GC at the same time on systems with limited cores and memory bandwidth.

We did not implement this for MEMORY_PRESSURE GCs. This bug does that, which is important because a system may be swapping heavily at that moment. If that's the case this is very important for MEMORY_PRESSURE GCs, at the very least running them sequentially means less memory is touched at the same time during tracing which should cause less swapping.

(I thought I had implemented this with Bug 1629064 but did not check the right code paths.)

Comment 1

[Paul Bone [:pbone]]

Attachment: Bug 1715705 - Move memory pressure GC code to a new function r=smaug

Comment 2

[Paul Bone [:pbone]]

Attachment: Bug 1715705 - Ask the parent before doing a MEMORY_PRESSURE GC r=smaug

Depends on D117395

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7729e9dfb60a
Ask the parent before doing a MEMORY_PRESSURE GC r=smaug

Comment 4

[Marian-Vasile Laza]

Backed out changeset 7729e9dfb60a (Bug 1715705) for causing failures in browser_bug1204626.js
Backout link
Push with failures
Failure Log

Comment 5

[Calixte Denizet (:calixte)]

The patch maybe caused this crash:
https://crash-stats.mozilla.org/report/index/9f2247ea-4603-421d-8edd-8ec920210614.

Comment 6

[Paul Bone [:pbone]]

I've found the cause for the test failures. There's two relevant payload values for memory-pressure. low-memory and heap-minimize. Notifiers expect heap-minimize to complete synchronously, it's used in some of our tests and even in our memory reporter. So I'll keep that synchronous and leave low-memory to use the new behaviour.

Comment 7

[Paul Bone [:pbone]]

(In reply to Calixte Denizet (:calixte) from comment #5)

The patch maybe caused this crash:
https://crash-stats.mozilla.org/report/index/9f2247ea-4603-421d-8edd-8ec920210614.

If I'm looking at https://hg.mozilla.org/integration/autoland/rev/47cad7171df3 right it says my change was backed-out before that build was made. But we should still keep an eye on such crashes, I'm changing that code path and what I hope is that crashes move around to new signatures without increasing in frequency.

Thanks.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/978c481175d9
Ask the parent before doing a MEMORY_PRESSURE GC r=smaug

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/978c481175d9