Bug 84897

Summary: WebCore shouldn't call collectAllGarbage directly
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, ggaren, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Mark Hahnenberg 2012-04-25 15:39:23 PDT
Currently, GCController calls Heap::collectAllGarbage directly, which leads to an overload of collections as the timer in GCController and the timer in GCActivityCallback compete for collection time and fire independently. As a result, we end up doing almost 600 full collections during an in-browser run of SunSpider, or 20 full collections on a single load of TechCrunch. 

We can do better by preventing WebCore from calling collectAllGarbage directly and instead going through Heap::reportAbandonedObjectGraph, since that is what WebCore is trying to do--notify the Heap that a lot of garbage may have just been generated when we left a page.
Comment 1 Mark Hahnenberg 2012-04-25 16:13:27 PDT
Created attachment 138891 [details]
Patch
Comment 2 Mark Hahnenberg 2012-04-25 16:29:15 PDT
With this patch, we do 133 GCs in a run of in-browser SunSpider running 20 iterations. We used to do 712 collections using this same testing setup.

We also do 10 collections vs. 20 collections when loading TechCrunch.
Comment 3 Mark Hahnenberg 2012-04-25 16:30:03 PDT
I should also mention that neither our wall clock time nor our benchmark score for in-browser SunSpider are affected by this patch.
Comment 4 Build Bot 2012-04-25 16:46:36 PDT
Comment on attachment 138891 [details]
Patch

Attachment 138891 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12520704
Comment 5 Mark Hahnenberg 2012-04-25 16:55:23 PDT
Created attachment 138899 [details]
Patch
Comment 6 Geoffrey Garen 2012-04-25 18:22:16 PDT
Comment on attachment 138899 [details]
Patch

r=me
Comment 7 WebKit Review Bot 2012-04-25 22:07:26 PDT
Comment on attachment 138899 [details]
Patch

Clearing flags on attachment: 138899

Committed r115288: <http://trac.webkit.org/changeset/115288>
Comment 8 WebKit Review Bot 2012-04-25 22:07:32 PDT
All reviewed patches have been landed.  Closing bug.