Changeset 220322 in webkit


Ignore:
Timestamp:
Aug 5, 2017 9:43:37 PM (7 years ago)
Author:
fpizlo@apple.com
Message:

REGRESSION (r219895-219897): Number of leaks on Open Source went from 9240 to 235983 and is now at 302372
​https://bugs.webkit.org/show_bug.cgi?id=175083

Reviewed by Oliver Hunt.

Source/JavaScriptCore:

This fixes the leak by making MarkedBlock::specializedSweep call destructors when the block is empty,
even if we are using the pop path.

Also, this fixes HeapCellInlines.h to no longer include MarkedBlockInlines.h. That's pretty
important, since MarkedBlockInlines.h is the GC's internal guts - we don't want to have to recompile
the world just because we changed it.

Finally, this adds a new testing SPI for waiting for all VMs to finish destructing. This makes it
easier to debug leaks.

  • bytecode/AccessCase.cpp:
  • bytecode/PolymorphicAccess.cpp:
  • heap/HeapCell.cpp:

(JSC::HeapCell::isLive):

  • heap/HeapCellInlines.h:

(JSC::HeapCell::isLive): Deleted.

  • heap/MarkedAllocator.cpp:

(JSC::MarkedAllocator::tryAllocateWithoutCollecting):
(JSC::MarkedAllocator::endMarking):

  • heap/MarkedBlockInlines.h:

(JSC::MarkedBlock::Handle::specializedSweep):

  • jit/AssemblyHelpers.cpp:
  • jit/Repatch.cpp:
  • runtime/TestRunnerUtils.h:
  • runtime/VM.cpp:

(JSC::waitForVMDestruction):
(JSC::VM::~VM):

Source/WTF:

Adds a classic ReadWriteLock class. I wrote my own because I can never remember if the pthread one is
guaranted to bias in favor of writers or not.

  • WTF.xcodeproj/project.pbxproj:
  • wtf/Condition.h:

(WTF::ConditionBase::construct):
(WTF::Condition::Condition):

  • wtf/Lock.h:

(WTF::LockBase::construct):
(WTF::Lock::Lock):

  • wtf/ReadWriteLock.cpp: Added.

(WTF::ReadWriteLockBase::construct):
(WTF::ReadWriteLockBase::readLock):
(WTF::ReadWriteLockBase::readUnlock):
(WTF::ReadWriteLockBase::writeLock):
(WTF::ReadWriteLockBase::writeUnlock):

  • wtf/ReadWriteLock.h: Added.

(WTF::ReadWriteLockBase::ReadLock::tryLock):
(WTF::ReadWriteLockBase::ReadLock::lock):
(WTF::ReadWriteLockBase::ReadLock::unlock):
(WTF::ReadWriteLockBase::WriteLock::tryLock):
(WTF::ReadWriteLockBase::WriteLock::lock):
(WTF::ReadWriteLockBase::WriteLock::unlock):
(WTF::ReadWriteLockBase::read):
(WTF::ReadWriteLockBase::write):
(WTF::ReadWriteLock::ReadWriteLock):

Tools:

Leaks results are super confusing if leaks runs while some VMs are destructing. This calls a new SPI
to wait for VM destructions to finish before running the next test. This makes it easier to
understand leaks results from workers tests, and leads to fewer reported leaks.

  • DumpRenderTree/mac/DumpRenderTree.mm:

(runTest):

Location:
trunk
Files:
2 added
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r220318 r220322  
     12017-08-05  Filip Pizlo  <fpizlo@apple.com>
     2
     3        REGRESSION (r219895-219897): Number of leaks on Open Source went from 9240 to 235983 and is now at 302372
     4        https://bugs.webkit.org/show_bug.cgi?id=175083
     5
     6        Reviewed by Oliver Hunt.
     7       
     8        This fixes the leak by making MarkedBlock::specializedSweep call destructors when the block is empty,
     9        even if we are using the pop path.
     10       
     11        Also, this fixes HeapCellInlines.h to no longer include MarkedBlockInlines.h. That's pretty
     12        important, since MarkedBlockInlines.h is the GC's internal guts - we don't want to have to recompile
     13        the world just because we changed it.
     14       
     15        Finally, this adds a new testing SPI for waiting for all VMs to finish destructing. This makes it
     16        easier to debug leaks.
     17
     18        * bytecode/AccessCase.cpp:
     19        * bytecode/PolymorphicAccess.cpp:
     20        * heap/HeapCell.cpp:
     21        (JSC::HeapCell::isLive):
     22        * heap/HeapCellInlines.h:
     23        (JSC::HeapCell::isLive): Deleted.
     24        * heap/MarkedAllocator.cpp:
     25        (JSC::MarkedAllocator::tryAllocateWithoutCollecting):
     26        (JSC::MarkedAllocator::endMarking):
     27        * heap/MarkedBlockInlines.h:
     28        (JSC::MarkedBlock::Handle::specializedSweep):
     29        * jit/AssemblyHelpers.cpp:
     30        * jit/Repatch.cpp:
     31        * runtime/TestRunnerUtils.h:
     32        * runtime/VM.cpp:
     33        (JSC::waitForVMDestruction):
     34        (JSC::VM::~VM):
     35
    1362017-08-05  Mark Lam  <mark.lam@apple.com>
    237

  • trunk/Source/JavaScriptCore/bytecode/AccessCase.cpp

    r219981 r220322  
    4747#include "SlotVisitorInlines.h"
    4848#include "StructureStubInfo.h"
     49#include "SuperSampler.h"
    4950#include "ThunkGenerators.h"
    5051

  • trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp

    r218641 r220322  
    3939#include "StructureStubClearingWatchpoint.h"
    4040#include "StructureStubInfo.h"
     41#include "SuperSampler.h"
    4142#include <wtf/CommaPrinter.h>
    4243#include <wtf/ListDump.h>

  • trunk/Source/JavaScriptCore/heap/HeapCell.cpp

    r213773 r220322  
    2727#include "HeapCell.h"
    2828
     29#include "HeapCellInlines.h"
     30#include "MarkedBlockInlines.h"
    2931#include <wtf/PrintStream.h>
    3032
    3133namespace JSC {
     34
     35bool HeapCell::isLive()
     36{
     37    if (isLargeAllocation())
     38        return largeAllocation().isLive();
     39    auto& markedBlockHandle = markedBlock().handle();
     40    if (markedBlockHandle.isFreeListed())
     41        return !markedBlockHandle.isFreeListedCell(this);
     42    return markedBlockHandle.isLive(this);
     43}
    3244
    3345#if !COMPILER(GCC_OR_CLANG)

  • trunk/Source/JavaScriptCore/heap/HeapCellInlines.h

    r217429 r220322  
    2929#include "HeapCell.h"
    3030#include "LargeAllocation.h"
    31 #include "MarkedBlockInlines.h"
    3231#include "VM.h"
    3332
    3433namespace JSC {
    35 
    36 ALWAYS_INLINE bool HeapCell::isLive()
    37 {
    38     if (isLargeAllocation())
    39         return largeAllocation().isLive();
    40     auto& markedBlockHandle = markedBlock().handle();
    41     if (markedBlockHandle.isFreeListed())
    42         return !markedBlockHandle.isFreeListedCell(this);
    43     return markedBlockHandle.isLive(this);
    44 }
    4534
    4635ALWAYS_INLINE bool HeapCell::isLargeAllocation() const

  • trunk/Source/JavaScriptCore/heap/MarkedAllocator.cpp

    r220291 r220322  
    4040namespace JSC {
    4141
     42static constexpr bool tradeDestructorBlocks = true;
     43
    4244MarkedAllocator::MarkedAllocator(Heap* heap, Subspace* subspace, size_t cellSize)
    4345    : m_freeList(cellSize)
    … …  
    103105    }
    104106   
    105     if (Options::stealEmptyBlocksFromOtherAllocators()) {
     107    if (Options::stealEmptyBlocksFromOtherAllocators()
     108        && (tradeDestructorBlocks || !needsDestruction())) {
    106109        if (MarkedBlock::Handle* block = m_subspace->findEmptyBlockToSteal()) {
    107110            RELEASE_ASSERT(block->alignedMemoryAllocator() == m_subspace->alignedMemoryAllocator());
    … …  
    382385    // vectors.
    383386   
    384     m_empty = m_live & ~m_markingNotEmpty;
    385     m_canAllocateButNotEmpty = m_live & m_markingNotEmpty & ~m_markingRetired;
     387    if (!tradeDestructorBlocks && needsDestruction()) {
     388        ASSERT(m_empty.isEmpty());
     389        m_canAllocateButNotEmpty = m_live & ~m_markingRetired;
     390    } else {
     391        m_empty = m_live & ~m_markingNotEmpty;
     392        m_canAllocateButNotEmpty = m_live & m_markingNotEmpty & ~m_markingRetired;
     393    }
     394   
    386395    if (needsDestruction()) {
    387396        // There are some blocks that we didn't allocate out of in the last cycle, but we swept them. This

  • trunk/Source/JavaScriptCore/heap/MarkedBlockInlines.h

    r220175 r220322  
    224224        HeapCell* cell = reinterpret_cast_ptr<HeapCell*>(&block.atoms()[i]);
    225225
    226         if (destructionMode != BlockHasNoDestructors && emptyMode == NotEmpty)
     226        if (destructionMode != BlockHasNoDestructors)
    227227            destroy(cell);
    228228

  • trunk/Source/JavaScriptCore/jit/AssemblyHelpers.cpp

    r220219 r220322  
    3333#include "LinkBuffer.h"
    3434#include "MaxFrameExtentForSlowPathCall.h"
     35#include "SuperSampler.h"
    3536#include "ThunkGenerators.h"
    3637

  • trunk/Source/JavaScriptCore/jit/Repatch.cpp

    r219981 r220322  
    5959#include "StructureStubClearingWatchpoint.h"
    6060#include "StructureStubInfo.h"
     61#include "SuperSampler.h"
    6162#include "ThunkGenerators.h"
    6263#include <wtf/CommaPrinter.h>

  • trunk/Source/JavaScriptCore/runtime/TestRunnerUtils.h

    r206525 r220322  
    11/*
    2  * Copyright (C) 2013-2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    5555JS_EXPORT_PRIVATE void finalizeStatsAtEndOfTesting();
    5656
     57JS_EXPORT_PRIVATE void waitForVMDestruction();
     58
    5759} // namespace JSC

  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r220291 r220322  
    102102#include "StrongInlines.h"
    103103#include "StructureInlines.h"
     104#include "TestRunnerUtils.h"
    104105#include "ThunkGenerators.h"
    105106#include "TypeProfiler.h"
    … …  
    114115#include <wtf/CurrentTime.h>
    115116#include <wtf/ProcessID.h>
     117#include <wtf/ReadWriteLock.h>
    116118#include <wtf/SimpleStats.h>
    117119#include <wtf/StringPrintStream.h>
    … …  
    342344}
    343345
     346static StaticReadWriteLock s_destructionLock;
     347
     348void waitForVMDestruction()
     349{
     350    auto locker = holdLock(s_destructionLock.write());
     351}
     352
    344353VM::~VM()
    345354{
     355    auto destructionLocker = holdLock(s_destructionLock.read());
     356   
    346357    Gigacage::removeDisableCallback(gigacageDisabledCallback, this);
    347358    promiseDeferredTimer->stopRunningTasks();

  • trunk/Source/WTF/ChangeLog

    r220279 r220322  
     12017-08-05  Filip Pizlo  <fpizlo@apple.com>
     2
     3        REGRESSION (r219895-219897): Number of leaks on Open Source went from 9240 to 235983 and is now at 302372
     4        https://bugs.webkit.org/show_bug.cgi?id=175083
     5
     6        Reviewed by Oliver Hunt.
     7       
     8        Adds a classic ReadWriteLock class. I wrote my own because I can never remember if the pthread one is
     9        guaranted to bias in favor of writers or not.
     10
     11        * WTF.xcodeproj/project.pbxproj:
     12        * wtf/Condition.h:
     13        (WTF::ConditionBase::construct):
     14        (WTF::Condition::Condition):
     15        * wtf/Lock.h:
     16        (WTF::LockBase::construct):
     17        (WTF::Lock::Lock):
     18        * wtf/ReadWriteLock.cpp: Added.
     19        (WTF::ReadWriteLockBase::construct):
     20        (WTF::ReadWriteLockBase::readLock):
     21        (WTF::ReadWriteLockBase::readUnlock):
     22        (WTF::ReadWriteLockBase::writeLock):
     23        (WTF::ReadWriteLockBase::writeUnlock):
     24        * wtf/ReadWriteLock.h: Added.
     25        (WTF::ReadWriteLockBase::ReadLock::tryLock):
     26        (WTF::ReadWriteLockBase::ReadLock::lock):
     27        (WTF::ReadWriteLockBase::ReadLock::unlock):
     28        (WTF::ReadWriteLockBase::WriteLock::tryLock):
     29        (WTF::ReadWriteLockBase::WriteLock::lock):
     30        (WTF::ReadWriteLockBase::WriteLock::unlock):
     31        (WTF::ReadWriteLockBase::read):
     32        (WTF::ReadWriteLockBase::write):
     33        (WTF::ReadWriteLock::ReadWriteLock):
     34
    1352017-08-04  Matt Lewis  <jlewis3@apple.com>
    236

  • trunk/Source/WTF/WTF.xcodeproj/project.pbxproj

    r220214 r220322  
    3939                0FE1646A1B6FFC9600400E7C /* Lock.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0FE164681B6FFC9600400E7C /* Lock.cpp */; };
    4040                0FE4479C1B7AAA03009498EB /* WordLock.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0FE4479A1B7AAA03009498EB /* WordLock.cpp */; };
     41                0FEC3C5E1F368A9700F59B6C /* ReadWriteLock.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0FEC3C5C1F368A9700F59B6C /* ReadWriteLock.cpp */; };
    4142                0FFF19DC1BB334EB00886D91 /* ParallelHelperPool.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0FFF19DA1BB334EB00886D91 /* ParallelHelperPool.cpp */; };
    4243                14022F4118F5C3FC007FF0EB /* libbmalloc.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 14022F4018F5C3FC007FF0EB /* libbmalloc.a */; };
    … …  
    226227                0FEB3DD01BB7366B009D7AAD /* ParallelVectorIterator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ParallelVectorIterator.h; sourceTree = "<group>"; };
    227228                0FEC3C4F1F323C6800F59B6C /* CagedPtr.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CagedPtr.h; sourceTree = "<group>"; };
     229                0FEC3C5C1F368A9700F59B6C /* ReadWriteLock.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ReadWriteLock.cpp; sourceTree = "<group>"; };
     230                0FEC3C5D1F368A9700F59B6C /* ReadWriteLock.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ReadWriteLock.h; sourceTree = "<group>"; };
    228231                0FEC84AE1BD825310080FF74 /* GraphNodeWorklist.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = GraphNodeWorklist.h; sourceTree = "<group>"; };
    229232                0FEC84B01BDACD390080FF74 /* ScopedLambda.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ScopedLambda.h; sourceTree = "<group>"; };
    … …  
    910913                                0F725CAB1C50461600AD943A /* RangeSet.h */,
    911914                                0F87105916643F190090B0AD /* RawPointer.h */,
     915                                0FEC3C5C1F368A9700F59B6C /* ReadWriteLock.cpp */,
     916                                0FEC3C5D1F368A9700F59B6C /* ReadWriteLock.h */,
    912917                                0FDE87F61DFD07CC0064C390 /* RecursiveLockAdapter.h */,
    913918                                A8A472FE151A825B004123FF /* RedBlackTree.h */,
    … …  
    13381343                                0FE1646A1B6FFC9600400E7C /* Lock.cpp in Sources */,
    13391344                                0F60F32F1DFCBD1B00416D6C /* LockedPrintStream.cpp in Sources */,
     1345                                0FEC3C5E1F368A9700F59B6C /* ReadWriteLock.cpp in Sources */,
    13401346                                53534F2A1EC0E10E00141B2F /* MachExceptions.defs in Sources */,
    13411347                                A8A473E5151A825B004123FF /* MainThread.cpp in Sources */,

  • trunk/Source/WTF/wtf/CMakeLists.txt

    r220214 r220322  
    111111    RangeSet.h
    112112    RawPointer.h
     113    ReadWriteLock.h
    113114    RecursiveLockAdapter.h
    114115    RedBlackTree.h
    … …  
    242243    RandomDevice.cpp
    243244    RandomNumber.cpp
     245    ReadWriteLock.cpp
    244246    RefCountedLeakCounter.cpp
    245247    RunLoop.cpp

  • trunk/Source/WTF/wtf/Condition.h

    r218464 r220322  
    5050    // are unlikely to be affected by the cost of conversions, it is better to use MonotonicTime.
    5151    typedef ParkingLot::Time Time;
     52   
     53    void construct()
     54    {
     55        m_hasWaiters.store(false);
     56    }
    5257   
    5358    // Wait on a parking queue while releasing the given lock. It will unlock the lock just before
    … …  
    169174    }
    170175   
    171 protected:
     176private:
    172177    Atomic<bool> m_hasWaiters;
    173178};
    … …  
    178183    Condition()
    179184    {
    180         m_hasWaiters.store(false);
     185        construct();
    181186    }
    182187};

  • trunk/Source/WTF/wtf/Lock.h

    r209570 r220322  
    5353// Use Lock in instance variables.
    5454struct LockBase {
     55    void construct()
     56    {
     57        m_byte.store(0, std::memory_order_relaxed);
     58    }
     59   
    5560    void lock()
    5661    {
    … …  
    111116    }
    112117
    113 protected:
     118private:
    114119    friend struct TestWebKitAPI::LockInspector;
    115120   
    … …  
    137142    Lock()
    138143    {
    139         m_byte.store(0, std::memory_order_relaxed);
     144        construct();
    140145    }
    141146};

  • trunk/Tools/ChangeLog

    r220321 r220322  
     12017-08-05  Filip Pizlo  <fpizlo@apple.com>
     2
     3        REGRESSION (r219895-219897): Number of leaks on Open Source went from 9240 to 235983 and is now at 302372
     4        https://bugs.webkit.org/show_bug.cgi?id=175083
     5
     6        Reviewed by Oliver Hunt.
     7       
     8        Leaks results are super confusing if leaks runs while some VMs are destructing. This calls a new SPI
     9        to wait for VM destructions to finish before running the next test. This makes it easier to
     10        understand leaks results from workers tests, and leads to fewer reported leaks.
     11
     12        * DumpRenderTree/mac/DumpRenderTree.mm:
     13        (runTest):
     14
    1152017-08-05  Yoshiaki Jitsukawa  <Yoshiaki.Jitsukawa@sony.com>
    216

  • trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm

    r220311 r220322  
    20892089    if (gcBetweenTests)
    20902090        [WebCoreStatistics garbageCollectJavaScriptObjects];
     2091   
     2092    JSC::waitForVMDestruction();
    20912093
    20922094    fputs("#EOF\n", stderr);
Note: See TracChangeset for help on using the changeset viewer.