Title: [117183] trunk/Source/_javascript_Core
Revision
117183
Author
[email protected]
Date
2012-05-15 16:38:25 -0700 (Tue, 15 May 2012)

Log Message

Block freeing thread should not free blocks when we are actively requesting them
https://bugs.webkit.org/show_bug.cgi?id=86519

Reviewed by Geoffrey Garen.

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.

* heap/BlockAllocator.cpp:
(JSC::BlockAllocator::BlockAllocator): Initialize our new field.
(JSC::BlockAllocator::blockFreeingThreadMain): We check if we've seen any block requests recently.
If so, reset our flag and go back to sleep. We also don't bother with locking here. If we miss out
on an update, we'll see it when we wake up again.
* heap/BlockAllocator.h: Add new field to track whether or not we've received recent block requests.
(BlockAllocator):
(JSC::BlockAllocator::allocate): If we receive a request for a block, set our field that tracks
that to true. We don't bother locking since we assume that writing to a bool is atomic.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (117182 => 117183)


--- trunk/Source/_javascript_Core/ChangeLog	2012-05-15 23:36:02 UTC (rev 117182)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-05-15 23:38:25 UTC (rev 117183)
@@ -1,3 +1,34 @@
+2012-05-15  Mark Hahnenberg  <[email protected]>
+
+        Block freeing thread should not free blocks when we are actively requesting them
+        https://bugs.webkit.org/show_bug.cgi?id=86519
+
+        Reviewed by Geoffrey Garen.
+
+        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.
+
+        * heap/BlockAllocator.cpp:
+        (JSC::BlockAllocator::BlockAllocator): Initialize our new field.
+        (JSC::BlockAllocator::blockFreeingThreadMain): We check if we've seen any block requests recently.
+        If so, reset our flag and go back to sleep. We also don't bother with locking here. If we miss out 
+        on an update, we'll see it when we wake up again.
+        * heap/BlockAllocator.h: Add new field to track whether or not we've received recent block requests.
+        (BlockAllocator):
+        (JSC::BlockAllocator::allocate): If we receive a request for a block, set our field that tracks 
+        that to true. We don't bother locking since we assume that writing to a bool is atomic.
+
 2012-05-14  Luke Macpherson  <[email protected]>
 
         Introduce ENABLE_CSS_VARIABLES compile flag.

Modified: trunk/Source/_javascript_Core/heap/BlockAllocator.cpp (117182 => 117183)


--- trunk/Source/_javascript_Core/heap/BlockAllocator.cpp	2012-05-15 23:36:02 UTC (rev 117182)
+++ trunk/Source/_javascript_Core/heap/BlockAllocator.cpp	2012-05-15 23:38:25 UTC (rev 117183)
@@ -33,6 +33,7 @@
 
 BlockAllocator::BlockAllocator()
     : m_numberOfFreeBlocks(0)
+    , m_isCurrentlyAllocating(false)
     , m_blockFreeingThreadShouldQuit(false)
     , m_blockFreeingThread(createThread(blockFreeingThreadStartFunc, this, "_javascript_Core::BlockFree"))
 {
@@ -104,6 +105,11 @@
         if (m_blockFreeingThreadShouldQuit)
             break;
         
+        if (m_isCurrentlyAllocating) {
+            m_isCurrentlyAllocating = false;
+            continue;
+        }
+
         // Now process the list of free blocks. Keep freeing until half of the
         // blocks that are currently on the list are gone. Assume that a size_t
         // field can be accessed atomically.

Modified: trunk/Source/_javascript_Core/heap/BlockAllocator.h (117182 => 117183)


--- trunk/Source/_javascript_Core/heap/BlockAllocator.h	2012-05-15 23:36:02 UTC (rev 117182)
+++ trunk/Source/_javascript_Core/heap/BlockAllocator.h	2012-05-15 23:38:25 UTC (rev 117183)
@@ -56,6 +56,7 @@
 
     DoublyLinkedList<HeapBlock> m_freeBlocks;
     size_t m_numberOfFreeBlocks;
+    bool m_isCurrentlyAllocating;
     bool m_blockFreeingThreadShouldQuit;
     Mutex m_freeBlockLock;
     ThreadCondition m_freeBlockCondition;
@@ -64,6 +65,7 @@
 
 inline HeapBlock* BlockAllocator::allocate()
 {
+    m_isCurrentlyAllocating = true;
     MutexLocker locker(m_freeBlockLock);
     if (!m_numberOfFreeBlocks) {
         ASSERT(m_freeBlocks.isEmpty());
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to