Bug 12900 - Page tear-down forces garbage collection once per frame
Summary: Page tear-down forces garbage collection once per frame
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Geoffrey Garen
Depends on:
Reported: 2007-02-26 13:15 PST by Geoffrey Garen
Modified: 2007-07-20 16:02 PDT (History)
1 user (show)

See Also:

Pathological test case that demonstrates the value of this change (1.38 KB, text/html)
2007-07-20 14:36 PDT, Geoffrey Garen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2007-02-26 13:15:51 PST
Forcing garbage collection only once, at the end of tearing down the whole page, would save a lot of work. In the common case, there's no script execution that would benefit from the extra GC's between frame tear-downs. 

This seems like low-hanging performance fruit.
Comment 1 Geoffrey Garen 2007-02-26 22:33:10 PST
This is actually pretty tricky, and my first shot at it caused a performance regression (even though it reduced the number of garbage collections). 

I think the key to the regression was the Frame keepAlive timer. Since frame tear-down is delayed, there may be value to guaranteeing a collection right after the Frame has torn down, rather than at page load time.

I'm not sure there's a way to implement this optimization without eliminating the Frame keepAlive timer (and replacing it with a real RefPtr solution) or merging individual Frame keepAlive callbacks into a single callback, and collecting at the end of it.
Comment 2 Geoffrey Garen 2007-03-01 08:46:53 PST
I realized that a very simple way to fix this would be to schedule manual GC on a single zero-delay timer. That way, multiple manual GCs would coalesce. This would have the fortunate side-effect of doing GC when the stack was very small, reducing false positives in the conservative marking algorithm, and generally reducing marking overhead.

I tried this and it reduced the number of GCs during the PLT by 2/3. In the PLT, that didn't affect performance one way or the other. I suspect that's because there are very few live objects in the PLT when we GC.

In the pathological case of closing a tab containing many frames while many other JS-heavy tabs were open, I bet this would help a lot.

Not a priority right now, though, since it doesn't help page load performance.

(Sorry, but I lost the patch. It just changed Collector::collect() calls in WebCore to KJSProxy::garbageCollectSoon() calls. It implemented garbageCollectSoon() with GarbageCollectTimer : public TimerBase, that implemented fire() as Collector::collect().)
Comment 3 Geoffrey Garen 2007-07-20 14:36:30 PDT
Created attachment 15608 [details]
Pathological test case that demonstrates the value of this change
Comment 4 Geoffrey Garen 2007-07-20 16:02:28 PDT
Committed revision 24493.