Bug 86519 – Block freeing thread should not free blocks when we are actively requesting them

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

[Mark Hahnenberg]

The block freeing thread shoots us in the foot if it decides to run while we're actively requesting blocks and returning them. This situation can arise when there is a lot of copying collection going on in steady state. We allocate a large swath of pages to copy into, then we return all the newly free old pages to the BlockAllocator. In this state, if the block freeing thread wakes up in between collections (which is more likely than it waking up during a collection) and frees half of these pages, they will be needed almost immediately during the next collection, causing a storm of VM allocations which we know are going to be very slow.

What we'd like is for when things have quieted down the block freeing thread can then return memory to the OS. Usually this will be when a page has fully loaded and has a low allocation rate. In this situation, our opportunistic collections will only be running at least every few seconds, thus the extra time spent doing VM allocations won't matter nearly as much as, say, while a page is loading.

[Mark Hahnenberg]

Created attachment 142045 [details]
Patch

[Geoffrey Garen]

Comment on attachment 142045 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142045&action=review

r=me

> Source/JavaScriptCore/heap/BlockAllocator.h:69
> +    m_isCurrentlyAllocating = true;
>      MutexLocker locker(m_freeBlockLock);

Let's put this assignment inside the lock, to guarantee proper synchronization.

[Mark Hahnenberg]

Created attachment 142069 [details]
Benchmark results

Mostly neutral.

[Mark Hahnenberg]

<rdar://problem/11458880>

[WebKit Review Bot]

Comment on attachment 142045 [details]
Patch

Clearing flags on attachment: 142045

Committed r117183: <http://trac.webkit.org/changeset/117183>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mark Hahnenberg]

Reopening because I wasn't paying attention and committed the version that doesn't lock before setting the flag :-(

[Mark Hahnenberg]

Committed r117201: <http://trac.webkit.org/changeset/117201>