Andrew said this shouldn't be too hard. During the XPConnect black marking hook, we would iterate over all windows, check if the window is active, find the context for the window, and then mark its global object black.

I can look at this next week, after I finish my current batch of CC micro-optimizations, if you'd like.

Cool!

Minor variation on bug 735504. This bug should obsolete that one, except for windows that are newly created in between GCs.

Though currently I only run that code once after a GC, so I suppose it is pretty redundant.

This is crashy, but I think the problem is just that I need to null check what I pass to JS_CALL_OBJECT_TRACER.

I added the null check, and renamed the function. Ideally, we'd convert all of the cleanupJS functionality to TRACE_JS instead of unmark gray, and then move it into this function, so I wanted to give it a more generally name. https://tbpl.mozilla.org/?tree=Try&rev=b29f541080f2

Does nsGlobalWindow::GetWindowsTable(); really return only certainly alive windows?

No, it returns them all, so I filter out those that don't have doc shells. I'm following the logic listed in the comment in CollectWindowReports that is used for about:memory: http://mxr.mozilla.org/mozilla-central/source/dom/base/nsWindowMemoryReporter.cpp#92 The code there implies that windows with doc shells must be "active" ("currently either displayed in an active tab, or a child of such a window") or in the fastback cache, if they have doc shells. Maybe I'm wrong about that being the right thing to do. It is still pretty crashy, so something isn't right.

Ah, right. Windows with docshells should be alive.

recursion.js fails locally when run as part of the suite, which is what I saw on try, but passes when run by itself, or even in somewhat larger chunks of tests. Well, first it hangs on a test that is showing up as a random orange on TBPL (I don't have the bug number handy), but if I disable that then I get the recursion failures.

Also, on OSX, I don't get any Mochitest failures, whereas I got a number of them for Linux64, mostly in WebGL.

Bill suggested that I check that every object that I was marking black was the object of a context we were going to mark gray. It turns out that this was not the case. From mrbkap in IRC, I found out that the global object of an inner window is not the same as the global object of its context (but is the same for the outer window). I don't know why, but grabbing the global from the context seems to at least not trigger the assertion on a test case that was breaking before. I'm not sure if having this debug code is worthwhile to actually land, as I was just making a silly mistake. I'm not sure what broke exactly, but maybe the JS object was dead or something.

Getting the global object out of the context takes an annoying number of null checks (which I failed to do in the previous version of my patch), so I went with this approach, where we instead grab the global object directly from the window, but only for outer windows. A slightly earlier version of this was causing a random orange to happen much more frequently, but it seems okay now. Either the check I added to disable the optimization on shutdown fixed it, or updating my tree fixed it. I just need to confirm that removing some debugging code I added won't mess things up.

I didn't notice any particular speed difference on a simple test case I tried, and it didn't actually help with the test case in bug 717500 unfortunately. It did get rid of those annoying ChromeWindow JS blobs I was seeing in the CC graph.

(In reply to Andrew McCreight [:mccr8] from comment #15) > I didn't notice any particular speed difference on a simple test case I > tried, and it didn't actually help with the test case in bug 717500 > unfortunately. It did get rid of those annoying ChromeWindow JS blobs I was > seeing in the CC graph. Nevertheless, this patch should at least give us some peace of mind when we start calling UnmarkGray more often.

Comment on attachment 608213 [details] [diff] [review] actually works Try run looked good. I'm open to suggestions regarding the name and location of dom::TraceBlackJS. I basically went with the least creative possible options. I removed the debugging code I added that checks that each object you mark is the global object of a context, but I could add that back in if it makes sense.

Comment on attachment 608213 [details] [diff] [review] actually works >+mozilla::dom::TraceBlackJS(JSTracer *trc) >+{ >+ if (!nsCCUncollectableMarker::sGeneration) >+ return; if (expr) { stmt; } >+ >+ // Mark globals of active windows black. >+ nsGlobalWindow::WindowByIdTable* windowsById = >+ nsGlobalWindow::GetWindowsTable(); >+ if (windowsById) >+ windowsById->Enumerate(TraceActiveWindowGlobal, trc); Ditto >+void TraceBlackJS(JSTracer* trc); aParameterName

review comments addressed, plus some other minor style fiddling. https://hg.mozilla.org/integration/mozilla-inbound/rev/bb496ca24eef