Switching to MMgc obsoletes the old heuristic. In bug 391211, the latest patch has JS_MaybeGC doing a collection only if GC_ZEAL is turned on; we can do better than that. Offhand Brendan suggested using the amount of memory (number of pages) committed, which should be quite easy to track if MMgc doesn't already.

What is the function of JS_MaybeGC? Is it a hook for opportunistic GC? Because MMgc is incremental we already have a huerstic to start an incremental GC if we've consumed some portion of existing memory. Perhaps JS_MaybeGC should just perform an incremental marking increment if we're in the incremental marking phase.

Hey Tommy, JS_MaybeGC is dumb as dirt. It's a hook for opportunistic GC but SpiderMonkey has had no incremental GC. ActionMonkey is not ready for incremental AFAIK. So for now we probably want JS_MaybeGC to keep trying to figure out when to do a global GC if there's some evidence it will collect a bunch of garbage. That heuristic MMgc uses to decide when to start an incremental GC sounds like it could be scaled to work. /be

Sounds fine, maybe it could be written in a forward thinking way so that it calls IncrementalMark if we're in incremental mode.

(In reply to comment #2) > That heuristic MMgc uses to decide when to start an incremental GC sounds like > it could be scaled to work. Yep. This code appears in GC::AllocBlock and looks about right: uint64 now = GetPerformanceCounter(); if(!marking && !collecting && heapSizeAtLastAlloc > collectThreshold && now - lastSweepTicks > kMarkSweepBurstTicks && heapSizeAtLastAlloc < heap->GetTotalHeapSize()) { if(incremental) StartIncrementalMark(); else Collect(); } A separate heuristic is used in GC::AllocBlockIncremental. Maybe we can factor both into a single, new, public method.

This patch sits atop 4-5 other patches that haven't landed yet.

This patch sits on top of the patch in bug 395962.

Looks good to me.

Comment on attachment 293687 [details] [diff] [review] patch for actionmonkey-tamarin, v1 Certianly nothing obviously wrong with it and harmless until you call MaybeGC, so + for that. kind of wish there was a good one-stop-shop for what the heuristic values all are (thresholds,etc) but thats not what this bug is about.

Pushed changeset c138b1ab5626 to tamarin-central.

Pushed changeset f4b554fa4d29 to actionmonkey.