Comment on attachment 600877[details][diff][review]
patch
Review of attachment 600877[details][diff][review]:
-----------------------------------------------------------------
Makes sense. We really have no idea how much garbage a GC could have released, so we should err on the side of caution.
::: dom/base/nsJSEnvironment.cpp
@@ +180,5 @@
> static PRUint32 sForgetSkippableBeforeCC = 0;
> static PRUint32 sPreviousSuspectedCount = 0;
>
> static bool sCleanupSinceLastGC = true;
> +static bool sNeedsFullCC = false;
Do you need to set this to false in that init function, too?

(In reply to Andrew McCreight [:mccr8] from comment #6)
> > +static bool sNeedsFullCC = false;
>
> Do you need to set this to false in that init function, too?
I could do that, just to be consistent.
(Someone should cleanup nsJSEnvironment.cpp)

Boo. Then it is about something else.
We should still use the patch.
Could you check if GC and/or CC runs while running the test.
Set javascript.options.mem.log to true and check error console Messages.

With this patch (well, and my patch in bug 728460), I'm seeing about 80% of CCs being triggered by this force CC. It kind of makes the CC hostage to the GC if we want to try to call the CC less. Kind of a shame, but I think this patch is probably the way to go, because otherwise you'll hit situations like this that are problematic.

This probably belongs in another bug, but another approach to getting the GC to trigger a CC would be to dump all wrapped natives (etc.) into the purple buffer at the conclusion of a GC. Then we could filter out everything held by a marked object using the normal methods. That would probably work for this particular case, where it is allocating a huge number of cross-compartment cycles.

(In reply to Scoobidiver from comment #29)
> Bug 730853 didn't land in 12.0 so it's not fully fixed.
Given the original risk evaluation that this would only occur in rare cases, untracking for 12 since we wouldn't take any more forward change for this.