1749797 - The first cycle collection always triggers a full non-incremental GC

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task)

Description

[Jon Coppeard (:jonco)]

The GC has a flag to say whether the gray bits are valid, and this is set to false if we hit OOM while unmarking gray things. Currently this is initially false. This has the effect of forcing a full non-incremental GC when we do the first CC, to fix the invalid gray bits. This shows up in cold pageload profiles.

I'm wondering if this is necessary. We don't clear this flag when a new zone is created or when new allocations happen after this first full GC. After that point it's only ever set to false on OOM. So maybe this could be true initially, which would get rid of this non-incremental GC.

Comment 1

[Jon Coppeard (:jonco)]

Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7140c4672d05a7da0f570dab80d2c21fafc450a

Comment 2

[Jon Coppeard (:jonco)]

Andrew, do you think this is feasible? Is there some other reason this is necessary?

Comment 3

[Andrew McCreight [:mccr8]]

I think the idea is that we don't want to run the CC until we've run the first GC, and that under normal circumstances we should run the first GC before the first CC so we won't actually hit it. Are you not seeing that? Maybe we need to delay the initial CC more explicitly until after the first GC is run?

Comment 4

[Paul Bone [:pbone]]

(In reply to Andrew McCreight [:mccr8] from comment #3)

I think the idea is that we don't want to run the CC until we've run the first GC, and that under normal circumstances we should run the first GC before the first CC so we won't actually hit it. Are you not seeing that? Maybe we need to delay the initial CC more explicitly until after the first GC is run?

If that's the intention I'd prefer more explicit flags in CCGCScheduler that capture that intention.

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Andrew McCreight [:mccr8] from comment #3)
I'm seeing an initial incremental GC (that doesn't collect all zones), followed by a second non-incremental all-zones GC when the CC runs for the first time.

The problem is that the gray bits valid flag is not set until the first GC that collects all zones (actually the atoms zone is not counted but that's not important) so the first GC doesn't set it.

I think the idea is that we don't want to run the CC until we've run the first GC

What's the reason for this? Sure, the CC will not be able to collect any cycles created through JS zones since that zone was GCed but this is true after the first GC too (and there's no suggestion that we force a full GC every time the CC runs). Do we expect a lot of cycles to be created during startup perhaps?

I think we should either set the gray bits valid flag to true initially, or have the scheduler make the first GC collect all-zones.

Comment 6

[Jon Coppeard (:jonco)]

Here's a profile: https://share.firefox.dev/3I5VKbP

See the second GC of 87ms.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: Bug 1749797 - Treat gray bits as initially valid r=sfink!,mccr8!

Previously the 'gray mark bits are valid state' was initially false, and set to
true after the first full GC. But there's no reason that the gray state is
invalid initially. The state is only marked as invalid when we abort gray
unmarking due to OOM, which does lead to an actually incorrect state where some
things are marked gray when they should be black. The initial state of cells
being marked white is not invalid, although cycles through white cells cannot
be collected at this time.

This patch has no impact on AWSY.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5907e9a7bca2
Treat gray bits as initially valid r=sfink,mccr8

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/5907e9a7bca2