Closed Bug 712704 Opened 8 years ago Closed 6 years ago

improve the interaction of GC and CC scheduling

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: mccr8, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [Snappy:P2])

The current interaction of GC and CC scheduling could probably be improved.

The logic for the GC triggering the CC lives in DOMGCFinishedCallback, in nsJSEnvironment.cpp:

if (sGCTimer) { // if a new GC was triggered while we were GCing
  nsJSContext::KillGCTimer(); // cancel the GC
  if (comp) { // if the GC was compartmental
    nsJSContext::PokeGC(gcwhy::POST_COMPARTMENT); // do a new GC
    nsJSContext::KillCCTimer(); // cancel the pending CC, if any
  }
} else {
  // in this condition there was no additional GC scheduled while we GCed
  if (!comp) { // if it was a full GC
    sGCHasRun = true; // set a flag saying we've run the GC once
    nsJSContext::PokeCC(ccwhy::POST_GC); // schedule a CC
  }
}

I don't understand why sGCTimer guards anything but KillGCTimer.  Why do we only do a full GC and cancel the pending CC after a compartmental GC if the GC timer triggered while we were GCing?  Why do we only say that we've run a GC already and poke the CC if there was no pending GC?  sGCHasRun seems somewhat archaic, but I guess it is useful to delay the first CC (which is typically pretty long) until startup has settled out.

The logic for the CC triggering the GC is in nsJSContext::CycleCollectNow:
  if (sCCollectedWaitingForGC > 250) {
    PokeGC(gcwhy::CC_WAITING);
  }

sCCollectedWaitingForGC is a static variable that is incremented by the number of objects the CC frees, and resets to 0 every time the GC runs.  This is a fairly crude metric because all C++ objects count towards this, even though they are mostly freed by the CC, and thus not actually waiting for the GC.  A large DOM being freed will trigger this, even if it contains no references to JS (though maybe that is not a realistic scenario).  It wouldn't be too hard to compute the number of JS objects that are waiting to be freed and use that instead, which would undercount slightly (by not including wrapped natives) but would be a little closer to the mark.  Secondly, we should look into whether the threshold of 250 is still reasonable.  It was introduced fairly recently, in bug 637206, so it shouldn't be too out of date, but it doesn't hurt to check.

On a larger scale, it seems a little problematic that only the CC can kill the cycling between CC and GC.  In theory, it should only take GC->CC->GC to completely kill all cross-domain cycles.  First GC marks objects gray, then the CC kills the C++ half of the cycle and marks wrappers for death, then the second GC kills off the JS part of the cycle, as well as wrappers and wrapped natives.  But because the GC always triggers a CC, we end up doing another CC.  Maybe we could track the reason a GC or CC is triggered, and if we can figure out that we've just been running a cross-domain cycle killing phase, we don't need to trigger that last CC.  Of course, in practice these cycles may die hard and need that extra CC, but we could at least look at it.
Depends on: 727965
Whiteboard: [Snappy] → [Snappy:P2]
Depends on: 731661
Depends on: 730853
No longer blocks: 698919
Blocks: 698919
This bug is a too non-specific to be useful.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.