740523 - Mark likely-leaked windows and documents as black for most CCs

Status: NEW

(Core :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

So we have lots of cool optimizations to keep us from traversing active windows and their child objects.  But all of these optimizations are useless as soon as we leak a window, whether due to a bug in Firefox or an add-on.  Then we traverse all of the window's objects on every CC.

In the case when our CC times are too long, perhaps we should stop traversing these likely-leaked windows' objects.  We can say, if CC times are longer than X and a window has survived Y CCs since it was destroyed, mark it as black for most CCs.

We can then do a full CC, with no pre-emptive black marking, upon idle, or once every 5 minutes, or something.

Comment 1

[Justin Lebar (not reading bugmail)]

This bug might mitigate a lot of the jank we see due to zombie compartments.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

Something like this could work. Try to first CC possible zombie
reasonable aggressively, but if that doesn't help, do it more rarely.
But, at some point try to collect all the zombies.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip 2

This is a bit better. static svg-as-image documents don't end up to the
CC graph after loading.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Hmm, that isn't quite good. if (mHasHadScriptHandlingObject) check removes also all the
XBL documents from zombie list.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: WIP3

Don't mark resource documents (svg-as-image and others) as zombies. They
are handled pretty differently.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Testing this now locally. Unfortunately I get zombies quite rarely.

I wonder if I should rename nsZombieCycleCollectable to PossibleZombieCycleCollectable.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Hmm, I'm leaking, badly.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 611908 [details] [diff] [review]
WIP3

I don't know yet what causes the leak, but I assume it is this patch.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Interesting. If I reload facebook, something keeps the document alive long enough so that
it ends up being a zombie, and then, I think, because the patch doesn't try to collect it always
it ends up staying alive.
I guess simpler, and more conservative patch is needed. Try to collect X (X ~ 10 ?) times, and if
the document is still alive, mark it to be a possible zombie.

Comment 10

[Justin Lebar (not reading bugmail)]

> Interesting. If I reload facebook, something keeps the document alive long enough so that
> it ends up being a zombie, and then, I think, because the patch doesn't try to collect it always
> it ends up staying alive.

You may need a heuristic like ghost windows have, where if you have some FB page open, you try to collect all FB pages.

Or alternatively, can you tell whether the window has any content pointers to it?

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 12

[Marco Castelluccio [:marco]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.