12900 2007-02-26 13:15:51 -0800 Page tear-down forces garbage collection once per frame 2007-07-20 16:02:28 -0700 1 1 1 Unclassified WebKit WebCore JavaScript 523.x (Safari 3) Mac OS X 10.4 RESOLVED FIXED P2 Normal --- 1 ggaren ggaren ejalbert oldest_to_newest 21309 0 ggaren 2007-02-26 13:15:51 -0800 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. 21288 1 ggaren 2007-02-26 22:33:10 -0800 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. 20783 2 ggaren 2007-03-01 08:46:53 -0800 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().) 4170 3 15608 ggaren 2007-07-20 14:36:30 -0700 Created attachment 15608 Pathological test case that demonstrates the value of this change 4156 4 ggaren 2007-07-20 16:02:28 -0700 Committed revision 24493. 15608 2007-07-20 14:36:30 -0700 2007-07-20 14:36:30 -0700 Pathological test case that demonstrates the value of this change scratch.html text/html 1415 ggaren PHA+VGhpcyB0ZXN0IG1pbWljcyBhIHdvcnN0IGNhc2Ugc2NlbmFyaW8gb2YgaGF2aW5nIG1hbnkg bGl2ZSBKUyBvYmplY3RzIChwZXJoYXBzCmluIHRhYnMpIGFuZCB0aGVuIG5hdmlnYXRpbmcgYXdh eSBmcm9tIGEgcGFnZSB3aXRoIG1hbnkgZnJhbWVzLjwvcD4KCjxwcmUgaWQ9ImNvbnNvbGUiPjwv cHJlPgoKPHNjcmlwdD4KZnVuY3Rpb24gbG9nKHMpCnsKICAgIGRvY3VtZW50LmdldEVsZW1lbnRC eUlkKCJjb25zb2xlIikuYXBwZW5kQ2hpbGQoZG9jdW1lbnQuY3JlYXRlVGV4dE5vZGUocykpOwp9 CgpmdW5jdGlvbiBnY1Byb3RlY3QobykKewogICAgaWYgKCFnY1Byb3RlY3QuYXJyYXkpCiAgICAg ICAgZ2NQcm90ZWN0LmFycmF5ID0gbmV3IEFycmF5OwogICAgZ2NQcm90ZWN0LmFycmF5LnB1c2go byk7Cn0KCmZ1bmN0aW9uIGFkZElmcmFtZShlbGVtZW50KQp7CiAgICB2YXIgaWZyYW1lID0gZWxl bWVudC5vd25lckRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoImlmcmFtZSIpOwogICAgZWxlbWVudC5h cHBlbmRDaGlsZChpZnJhbWUpOwp9CgpmdW5jdGlvbiBtYWluKCkKewogICAgLy8gc2ltdWxhdGUg YSBidW5jaCBvZiBsaXZlIEphdmFTY3JpcHQgb2JqZWN0cwogICAgZm9yICh2YXIgaSA9IDA7IGkg PCAxNTAwMDsgaSsrKSB7IC8vPgogICAgICAgIGdjUHJvdGVjdCh7eDoge30sIHk6IHt9LCB6OiB7 fX0pOwogICAgfQogICAgCiAgICB2YXIgc3RhcnQgPSBuZXcgRGF0ZTsKICAgIGJ1aWxkSWZyYW1l KCk7CiAgICBkZXN0cm95SWZyYW1lKCk7CiAgICBsb2coIkVsYXBzZWQgdGVzdGluZyB0aW1lOiAi ICsgU3RyaW5nKChuZXcgRGF0ZSkudmFsdWVPZigpIC0gc3RhcnQudmFsdWVPZigpKSk7Cn0KCmZ1 bmN0aW9uIGJ1aWxkSWZyYW1lKCkKewogICAgdmFyIGlmcmFtZSA9IGRvY3VtZW50LmNyZWF0ZUVs ZW1lbnQoImlmcmFtZSIpOwogICAgaWZyYW1lLmlkID0gImlmcmFtZSI7CiAgICBpZnJhbWUuc3R5 bGUudmlzaWJpbGl0eSA9ICJoaWRkZW4iOwogICAgaWZyYW1lLnN0eWxlLndpZHRoID0gMDsKICAg IGlmcmFtZS5zdHlsZS5oZWlnaHQgPSAwOwogICAgZG9jdW1lbnQuYm9keS5hcHBlbmRDaGlsZChp ZnJhbWUpOwogICAgCiAgICAvLyBzaW11bGF0ZSBhIHBhZ2Ugd2l0aCBhIGJ1bmNoIG9mIGZyYW1l cwogICAgdmFyIGVsZW1lbnQgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgiaWZyYW1lIik7CiAg ICBmb3IgKHZhciBpID0gMDsgaSA8IDEwMDsgaSsrKSB7IC8vPgogICAgICAgIGFkZElmcmFtZShl bGVtZW50KTsKICAgIH0KfQoKZnVuY3Rpb24gZGVzdHJveUlmcmFtZSgpCnsKICAgIHZhciBpZnJh bWUgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgiaWZyYW1lIik7CiAgICBpZnJhbWUucGFyZW50 Tm9kZS5yZW1vdmVDaGlsZChpZnJhbWUpOwp9CgptYWluKCk7Cjwvc2NyaXB0Pgo=