Bug 84404 – We're collecting pathologically due to small allocations

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Hahnenberg]

On google.com we're constantly running full collections due to the fact that after a collection, the first allocation will always hit the slow path because there are no free lists in any of the blocks. This causes the GC timer to set itself and fire very quickly, causing another full collection and the resetting of all the free lists. We should not be collecting so aggressively. 

A way to fix this is to change Heap so that instead of using a water mark to gauge whether or not a collection should be run, we track the number of bytes we've allocated since the last collection. When that hits a certain threshold, the Heap will recommend a collection be run. The Heap also notifies the GC activity timer of how many bytes have been allocated since the last collection, and the timer will ignore these notifications until the amount allocated exceeds a certain preset amount (currently 128K), after which it will schedule a timer to run a collection like it currently does. This prevents a sort of runaway timer that is rescheduled too eagerly.

This setup also puts the policy of future collection scheduling more squarely into the hands of the Heap and the GC activity timer, which will allow us to have more robust policies in the future.

[Mark Hahnenberg]

Created attachment 138008 [details]
Patch

[Geoffrey Garen]

Comment on attachment 138008 [details]
Patch

r=me, but please put your performance results in the ChangeLog.

[Mark Hahnenberg]

Committed r114698: <http://trac.webkit.org/changeset/114698>

[Yong Li]

I'm bisecting a sunspider regression. Now it appears to me this one makes a big impact. I'm testing on QNX armv7 with no DFG, no LLINT, no GGC, no GCActivityCallback implemented. Is it theoretically possible this commit can affect performance very much?

[Yong Li]

It seems the m_bytesAllocatedLimit (previously m_highWaterMark) is no longer twice of the new size.

    size_t newSize = size();
    if (fullGC) {
        m_lastFullGCSize = newSize;
        m_bytesAllocatedLimit = max(newSize, m_minBytesPerCycle);

After I make it like this:
m_bytesAllocatedLimit = max(newSize * 2, m_minBytesPerCycle);

the regression disappears. I will do more testing to confirm this.