Changeset 182200 in webkit


Ignore:
Timestamp:
Mar 31, 2015 1:01:29 PM (9 years ago)
Author:
akling@apple.com
Message:

Logically empty WeakBlocks should not pin down their MarkedBlocks indefinitely.
<https://webkit.org/b/143210>

Reviewed by Geoffrey Garen.

Since a MarkedBlock cannot be destroyed until all the WeakBlocks pointing into it are gone,
we had a little problem where WeakBlocks with only null pointers would still keep their
MarkedBlock alive.

This patch fixes that by detaching WeakBlocks from their MarkedBlock once a sweep discovers
that the WeakBlock contains no pointers to live objects. Ownership of the WeakBlock is passed
to the Heap, which will sweep the list of these detached WeakBlocks as part of a full GC,
destroying them once they're fully dead.

This allows the garbage collector to reclaim the 64kB MarkedBlocks much sooner, and resolves
a mysterious issue where doing two full garbage collections back-to-back would free additional
memory in the second collection.

Management of detached WeakBlocks is implemented as a Vector<WeakBlock*> in Heap, along with
an index of the next block in that vector that needs to be swept. The IncrementalSweeper then
calls into Heap::sweepNextLogicallyEmptyWeakBlock() to sweep one block at a time.

  • heap/Heap.h:
  • heap/Heap.cpp:

(JSC::Heap::collectAllGarbage): Add a final pass where we sweep the logically empty WeakBlocks
owned by Heap, after everything else has been swept.

(JSC::Heap::notifyIncrementalSweeper): Set up an incremental sweep of logically empty WeakBlocks
after a full garbage collection ends. Note that we don't do this after Eden collections, since
they are unlikely to cause entire WeakBlocks to go empty.

(JSC::Heap::addLogicallyEmptyWeakBlock): Added. Interface for passing ownership of a WeakBlock
to the Heap when it's detached from a WeakSet.

(JSC::Heap::sweepAllLogicallyEmptyWeakBlocks): Helper for collectAllGarbage() that sweeps all
of the logically empty WeakBlocks owned by Heap.

(JSC::Heap::sweepNextLogicallyEmptyWeakBlock): Sweeps one logically empty WeakBlock if needed
and updates the next-logically-empty-weak-block-to-sweep index.

(JSC::Heap::lastChanceToFinalize): call sweepAllLogicallyEmptyWeakBlocks() here, since there
won't be another chance after this.

  • heap/IncrementalSweeper.h:

(JSC::IncrementalSweeper::hasWork): Deleted.

  • heap/IncrementalSweeper.cpp:

(JSC::IncrementalSweeper::fullSweep):
(JSC::IncrementalSweeper::doSweep):
(JSC::IncrementalSweeper::sweepNextBlock): Restructured IncrementalSweeper a bit to simplify
adding a new sweeping stage for the Heap's logically empty WeakBlocks. sweepNextBlock() is
changed to return a bool (true if there's more work to be done.)

  • heap/WeakBlock.cpp:

(JSC::WeakBlock::sweep): This now figures out if the WeakBlock is logically empty, i.e doesn't
contain any pointers to live objects. The answer is stored in a new SweepResult member.

  • heap/WeakBlock.h:

(JSC::WeakBlock::isLogicallyEmptyButNotFree): Added. Can be queried after a sweep to determine
if the WeakBlock could be detached from the MarkedBlock.

(JSC::WeakBlock::SweepResult::SweepResult): Deleted in favor of initializing member variables
when declaring them.

Location:
trunk/Source/JavaScriptCore
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r182198 r182200  
     12015-03-31  Andreas Kling  <akling@apple.com>
     2
     3        Logically empty WeakBlocks should not pin down their MarkedBlocks indefinitely.
     4        <https://webkit.org/b/143210>
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Since a MarkedBlock cannot be destroyed until all the WeakBlocks pointing into it are gone,
     9        we had a little problem where WeakBlocks with only null pointers would still keep their
     10        MarkedBlock alive.
     11
     12        This patch fixes that by detaching WeakBlocks from their MarkedBlock once a sweep discovers
     13        that the WeakBlock contains no pointers to live objects. Ownership of the WeakBlock is passed
     14        to the Heap, which will sweep the list of these detached WeakBlocks as part of a full GC,
     15        destroying them once they're fully dead.
     16
     17        This allows the garbage collector to reclaim the 64kB MarkedBlocks much sooner, and resolves
     18        a mysterious issue where doing two full garbage collections back-to-back would free additional
     19        memory in the second collection.
     20
     21        Management of detached WeakBlocks is implemented as a Vector<WeakBlock*> in Heap, along with
     22        an index of the next block in that vector that needs to be swept. The IncrementalSweeper then
     23        calls into Heap::sweepNextLogicallyEmptyWeakBlock() to sweep one block at a time.
     24
     25        * heap/Heap.h:
     26        * heap/Heap.cpp:
     27        (JSC::Heap::collectAllGarbage): Add a final pass where we sweep the logically empty WeakBlocks
     28        owned by Heap, after everything else has been swept.
     29
     30        (JSC::Heap::notifyIncrementalSweeper): Set up an incremental sweep of logically empty WeakBlocks
     31        after a full garbage collection ends. Note that we don't do this after Eden collections, since
     32        they are unlikely to cause entire WeakBlocks to go empty.
     33
     34        (JSC::Heap::addLogicallyEmptyWeakBlock): Added. Interface for passing ownership of a WeakBlock
     35        to the Heap when it's detached from a WeakSet.
     36
     37        (JSC::Heap::sweepAllLogicallyEmptyWeakBlocks): Helper for collectAllGarbage() that sweeps all
     38        of the logically empty WeakBlocks owned by Heap.
     39
     40        (JSC::Heap::sweepNextLogicallyEmptyWeakBlock): Sweeps one logically empty WeakBlock if needed
     41        and updates the next-logically-empty-weak-block-to-sweep index.
     42
     43        (JSC::Heap::lastChanceToFinalize): call sweepAllLogicallyEmptyWeakBlocks() here, since there
     44        won't be another chance after this.
     45
     46        * heap/IncrementalSweeper.h:
     47        (JSC::IncrementalSweeper::hasWork): Deleted.
     48
     49        * heap/IncrementalSweeper.cpp:
     50        (JSC::IncrementalSweeper::fullSweep):
     51        (JSC::IncrementalSweeper::doSweep):
     52        (JSC::IncrementalSweeper::sweepNextBlock): Restructured IncrementalSweeper a bit to simplify
     53        adding a new sweeping stage for the Heap's logically empty WeakBlocks. sweepNextBlock() is
     54        changed to return a bool (true if there's more work to be done.)
     55
     56        * heap/WeakBlock.cpp:
     57        (JSC::WeakBlock::sweep): This now figures out if the WeakBlock is logically empty, i.e doesn't
     58        contain any pointers to live objects. The answer is stored in a new SweepResult member.
     59
     60        * heap/WeakBlock.h:
     61        (JSC::WeakBlock::isLogicallyEmptyButNotFree): Added. Can be queried after a sweep to determine
     62        if the WeakBlock could be detached from the MarkedBlock.
     63
     64        (JSC::WeakBlock::SweepResult::SweepResult): Deleted in favor of initializing member variables
     65        when declaring them.
     66
    1672015-03-31  Ryosuke Niwa  <rniwa@webkit.org>
    268
  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r181821 r182200  
    374374    m_objectSpace.lastChanceToFinalize();
    375375    releaseDelayedReleasedObjects();
     376
     377    sweepAllLogicallyEmptyWeakBlocks();
    376378}
    377379
     
    10031005    m_objectSpace.sweep();
    10041006    m_objectSpace.shrink();
     1007
     1008    sweepAllLogicallyEmptyWeakBlocks();
    10051009}
    10061010
     
    12411245    if (m_operationInProgress == EdenCollection)
    12421246        m_sweeper->addBlocksAndContinueSweeping(WTF::move(m_blockSnapshot));
    1243     else
     1247    else {
     1248        if (!m_logicallyEmptyWeakBlocks.isEmpty())
     1249            m_indexOfNextLogicallyEmptyWeakBlockToSweep = 0;
    12441250        m_sweeper->startSweeping(WTF::move(m_blockSnapshot));
     1251    }
    12451252}
    12461253
     
    14771484}
    14781485
     1486void Heap::addLogicallyEmptyWeakBlock(WeakBlock* block)
     1487{
     1488    m_logicallyEmptyWeakBlocks.append(block);
     1489}
     1490
     1491void Heap::sweepAllLogicallyEmptyWeakBlocks()
     1492{
     1493    if (m_logicallyEmptyWeakBlocks.isEmpty())
     1494        return;
     1495
     1496    m_indexOfNextLogicallyEmptyWeakBlockToSweep = 0;
     1497    while (sweepNextLogicallyEmptyWeakBlock()) { }
     1498}
     1499
     1500bool Heap::sweepNextLogicallyEmptyWeakBlock()
     1501{
     1502    if (m_indexOfNextLogicallyEmptyWeakBlockToSweep == WTF::notFound)
     1503        return false;
     1504
     1505    WeakBlock* block = m_logicallyEmptyWeakBlocks[m_indexOfNextLogicallyEmptyWeakBlockToSweep];
     1506
     1507    block->sweep();
     1508    if (block->isEmpty()) {
     1509        std::swap(m_logicallyEmptyWeakBlocks[m_indexOfNextLogicallyEmptyWeakBlockToSweep], m_logicallyEmptyWeakBlocks.last());
     1510        m_logicallyEmptyWeakBlocks.removeLast();
     1511        WeakBlock::destroy(block);
     1512    } else
     1513        m_indexOfNextLogicallyEmptyWeakBlockToSweep++;
     1514
     1515    if (m_indexOfNextLogicallyEmptyWeakBlockToSweep >= m_logicallyEmptyWeakBlocks.size()) {
     1516        m_indexOfNextLogicallyEmptyWeakBlockToSweep = WTF::notFound;
     1517        return false;
     1518    }
     1519
     1520    return true;
     1521}
     1522
    14791523} // namespace JSC
  • trunk/Source/JavaScriptCore/heap/Heap.h

    r181407 r182200  
    238238    void registerWeakGCMap(void* weakGCMap, std::function<void()> pruningCallback);
    239239    void unregisterWeakGCMap(void* weakGCMap);
     240
     241    void addLogicallyEmptyWeakBlock(WeakBlock*);
    240242
    241243private:
     
    331333    void markDeadObjects();
    332334
     335    void sweepAllLogicallyEmptyWeakBlocks();
     336    bool sweepNextLogicallyEmptyWeakBlock();
     337
    333338    bool shouldDoFullCollection(HeapOperation requestedCollectionType) const;
    334339    size_t sizeAfterCollect();
     
    393398
    394399    Vector<ExecutableBase*> m_compiledCode;
     400
     401    Vector<WeakBlock*> m_logicallyEmptyWeakBlocks;
     402    size_t m_indexOfNextLogicallyEmptyWeakBlockToSweep { WTF::notFound };
    395403   
    396404    RefPtr<GCActivityCallback> m_fullActivityCallback;
  • trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp

    r181486 r182200  
    6262void IncrementalSweeper::fullSweep()
    6363{
    64     while (hasWork())
    65         doWork();
     64    while (sweepNextBlock()) { }
    6665}
    6766
     
    7372void IncrementalSweeper::doSweep(double sweepBeginTime)
    7473{
    75     while (!m_blocksToSweep.isEmpty()) {
    76         sweepNextBlock();
    77 
     74    while (sweepNextBlock()) {
    7875        double elapsedTime = WTF::monotonicallyIncreasingTime() - sweepBeginTime;
    7976        if (elapsedTime < sweepTimeSlice)
     
    8885}
    8986
    90 void IncrementalSweeper::sweepNextBlock()
     87bool IncrementalSweeper::sweepNextBlock()
    9188{
    9289    while (!m_blocksToSweep.isEmpty()) {
     
    9996        block->sweep();
    10097        m_vm->heap.objectSpace().freeOrShrinkBlock(block);
    101         return;
     98        return true;
    10299    }
     100
     101    return m_vm->heap.sweepNextLogicallyEmptyWeakBlock();
    103102}
    104103
     
    148147}
    149148
    150 void IncrementalSweeper::sweepNextBlock()
     149bool IncrementalSweeper::sweepNextBlock()
    151150{
     151    return false;
    152152}
    153153
  • trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h

    r182068 r182200  
    4949
    5050    JS_EXPORT_PRIVATE virtual void doWork() override;
    51     void sweepNextBlock();
     51    bool sweepNextBlock();
    5252    void willFinishSweeping();
    5353
     
    5757    void scheduleTimer();
    5858    void cancelTimer();
    59     bool hasWork() const { return !m_blocksToSweep.isEmpty(); }
    6059   
    6160    Vector<MarkedBlock*>& m_blocksToSweep;
  • trunk/Source/JavaScriptCore/heap/WeakBlock.cpp

    r179361 r182200  
    8282        if (weakImpl->state() == WeakImpl::Deallocated)
    8383            addToFreeList(&sweepResult.freeList, weakImpl);
    84         else
     84        else {
    8585            sweepResult.blockIsFree = false;
     86            if (weakImpl->state() == WeakImpl::Live)
     87                sweepResult.blockIsLogicallyEmpty = false;
     88        }
    8689    }
    8790
  • trunk/Source/JavaScriptCore/heap/WeakBlock.h

    r179365 r182200  
    4949
    5050    struct SweepResult {
    51         SweepResult();
    5251        bool isNull() const;
    5352
    54         bool blockIsFree;
    55         FreeCell* freeList;
     53        bool blockIsFree { true };
     54        bool blockIsLogicallyEmpty { true };
     55        FreeCell* freeList { nullptr };
    5656    };
    5757
     
    6262
    6363    bool isEmpty();
     64    bool isLogicallyEmptyButNotFree() const;
    6465
    6566    void sweep();
     
    8586    SweepResult m_sweepResult;
    8687};
    87 
    88 inline WeakBlock::SweepResult::SweepResult()
    89     : blockIsFree(true)
    90     , freeList(0)
    91 {
    92     ASSERT(isNull());
    93 }
    9488
    9589inline bool WeakBlock::SweepResult::isNull() const
     
    141135}
    142136
     137inline bool WeakBlock::isLogicallyEmptyButNotFree() const
     138{
     139    return !m_sweepResult.isNull() && !m_sweepResult.blockIsFree && m_sweepResult.blockIsLogicallyEmpty;
     140}
     141
    143142} // namespace JSC
    144143
  • trunk/Source/JavaScriptCore/heap/WeakSet.cpp

    r179361 r182200  
    4545void WeakSet::sweep()
    4646{
    47     for (WeakBlock* block = m_blocks.head(); block; block = block->next())
     47    for (WeakBlock* block = m_blocks.head(); block;) {
     48        WeakBlock* nextBlock = block->next();
    4849        block->sweep();
     50        if (block->isLogicallyEmptyButNotFree()) {
     51            // If this WeakBlock is logically empty, but still has Weaks pointing into it,
     52            // we can't destroy it just yet. Detach it from the WeakSet and hand ownership
     53            // to the Heap so we don't pin down the entire 64kB MarkedBlock.
     54            m_blocks.remove(block);
     55            heap()->addLogicallyEmptyWeakBlock(block);
     56        }
     57        block = nextBlock;
     58    }
    4959
    5060    resetAllocator();
Note: See TracChangeset for help on using the changeset viewer.