On memory pressure, trigger a second GC if it would probably be productive

RESOLVED FIXED in mozilla30

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

(Blocks: 1 bug)

Trunk
mozilla30
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
When we finish a CC, we poke the GC if sCCollectedWaitingForGC exceeds some threshold.  But if we're doing a GC->CC combo due to getting a memory pressure event, in nsJSEnvironmentObserver::Observe, we probably want to do that second GC immediately.
(Assignee)

Comment 1

5 years ago
(Luke pointed out that we probably want that second GC on memory pressure.)
(Assignee)

Comment 2

5 years ago
This really just makes the memory pressure more useful, we could add it either way.
Blocks: 865959
No longer blocks: 936236
And that second GC will then trigger second (async) CC too.
(Assignee)

Comment 4

5 years ago
(In reply to Olli Pettay [:smaug] from comment #3)
> And that second GC will then trigger second (async) CC too.

Sure.  Hopefully it won't do much.  The hope of the second GC is that it will free a bunch of stuff the CC made into garbage.

try run: https://tbpl.mozilla.org/?tree=Try&rev=0b86aae5b34f
(Assignee)

Comment 5

5 years ago
Created attachment 8368656 [details] [diff] [review]
Do an extra GC on memory pressure if it would be productive.
Attachment #8368656 - Flags: review?(bugs)
https://hg.mozilla.org/mozilla-central/rev/06095fccf046
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
"Minimize memory usage" triggers 3 memory pressure events, precisely so that this GC/CC/GC dance gets done sufficiently. Could the number be reduced to 2 or 1 now?
(Assignee)

Comment 9

5 years ago
Reducing it to 2 should be okay.  In theory, 2 would have been okay before (in theory, 1 is okay now).  Trading a little jankiness for less weirdness in case of bugginess is probably the way to go, or we'll just have to tell people to hit the button multiple times, which defeats the point.
I was thinking that MMU happened when apps went into the background, but that's just vanilla memory-pressure. Yeah, jank after hitting the MMU button doesn't matter.
You need to log in before you can comment on or make changes to this bug.