⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 227906 in webkit


Ignore:
Timestamp:
Jan 31, 2018, 10:57:13 AM (9 years ago)
Author:
keith_miller@apple.com
Message:

Canonicalize aquiring the JSCell lock.
​https://bugs.webkit.org/show_bug.cgi?id=182320

Reviewed by Michael Saboff.

It's currently kinda annoying to figure out where
we aquire the a JSCell's lock. This patch adds a
helper to make it easier to grep...

  • bytecode/UnlinkedCodeBlock.cpp:

(JSC::UnlinkedCodeBlock::visitChildren):
(JSC::UnlinkedCodeBlock::setInstructions):
(JSC::UnlinkedCodeBlock::shrinkToFit):

  • runtime/ErrorInstance.cpp:

(JSC::ErrorInstance::finishCreation):
(JSC::ErrorInstance::materializeErrorInfoIfNeeded):
(JSC::ErrorInstance::visitChildren):

  • runtime/JSArray.cpp:

(JSC::JSArray::shiftCountWithArrayStorage):
(JSC::JSArray::unshiftCountWithArrayStorage):

  • runtime/JSCell.h:

(JSC::JSCell::cellLock):

  • runtime/JSObject.cpp:

(JSC::JSObject::visitButterflyImpl):
(JSC::JSObject::convertContiguousToArrayStorage):

  • runtime/JSPropertyNameEnumerator.cpp:

(JSC::JSPropertyNameEnumerator::visitChildren):

  • runtime/SparseArrayValueMap.cpp:

(JSC::SparseArrayValueMap::add):
(JSC::SparseArrayValueMap::remove):
(JSC::SparseArrayValueMap::visitChildren):

Location:
trunk/Source/JavaScriptCore
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r227898 r227906  
     12018-01-31  Keith Miller  <keith_miller@apple.com>
     2
     3        Canonicalize aquiring the JSCell lock.
     4        https://bugs.webkit.org/show_bug.cgi?id=182320
     5
     6        Reviewed by Michael Saboff.
     7
     8        It's currently kinda annoying to figure out where
     9        we aquire the a JSCell's lock. This patch adds a
     10        helper to make it easier to grep...
     11
     12        * bytecode/UnlinkedCodeBlock.cpp:
     13        (JSC::UnlinkedCodeBlock::visitChildren):
     14        (JSC::UnlinkedCodeBlock::setInstructions):
     15        (JSC::UnlinkedCodeBlock::shrinkToFit):
     16        * runtime/ErrorInstance.cpp:
     17        (JSC::ErrorInstance::finishCreation):
     18        (JSC::ErrorInstance::materializeErrorInfoIfNeeded):
     19        (JSC::ErrorInstance::visitChildren):
     20        * runtime/JSArray.cpp:
     21        (JSC::JSArray::shiftCountWithArrayStorage):
     22        (JSC::JSArray::unshiftCountWithArrayStorage):
     23        * runtime/JSCell.h:
     24        (JSC::JSCell::cellLock):
     25        * runtime/JSObject.cpp:
     26        (JSC::JSObject::visitButterflyImpl):
     27        (JSC::JSObject::convertContiguousToArrayStorage):
     28        * runtime/JSPropertyNameEnumerator.cpp:
     29        (JSC::JSPropertyNameEnumerator::visitChildren):
     30        * runtime/SparseArrayValueMap.cpp:
     31        (JSC::SparseArrayValueMap::add):
     32        (JSC::SparseArrayValueMap::remove):
     33        (JSC::SparseArrayValueMap::visitChildren):
     34
    1352018-01-31  Saam Barati  <sbarati@apple.com>
    236

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

    r226928 r227906  
    9595    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
    9696    Base::visitChildren(thisObject, visitor);
    97     auto locker = holdLock(*thisObject);
     97    auto locker = holdLock(thisObject->cellLock());
    9898    for (FunctionExpressionVector::iterator ptr = thisObject->m_functionDecls.begin(), end = thisObject->m_functionDecls.end(); ptr != end; ++ptr)
    9999        visitor.append(*ptr);
    … …  
    318318    ASSERT(instructions);
    319319    {
    320         auto locker = holdLock(*this);
     320        auto locker = holdLock(cellLock());
    321321        m_unlinkedInstructions = WTFMove(instructions);
    322322    }
    … …  
    393393void UnlinkedCodeBlock::shrinkToFit()
    394394{
    395     auto locker = holdLock(*this);
     395    auto locker = holdLock(cellLock());
    396396   
    397397    m_jumpTargets.shrinkToFit();

  • trunk/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h

    r226928 r227906  
    161161        createRareDataIfNecessary();
    162162        VM& vm = *this->vm();
    163         auto locker = lockDuringMarking(vm.heap, *this);
     163        auto locker = lockDuringMarking(vm.heap, cellLock());
    164164        unsigned size = m_rareData->m_regexps.size();
    165165        m_rareData->m_regexps.append(WriteBarrier<RegExp>(vm, this, r));
    … …  
    192192    {
    193193        VM& vm = *this->vm();
    194         auto locker = lockDuringMarking(vm.heap, *this);
     194        auto locker = lockDuringMarking(vm.heap, cellLock());
    195195        unsigned result = m_constantRegisters.size();
    196196        m_constantRegisters.append(WriteBarrier<Unknown>());
    … …  
    202202    {
    203203        VM& vm = *this->vm();
    204         auto locker = lockDuringMarking(vm.heap, *this);
     204        auto locker = lockDuringMarking(vm.heap, cellLock());
    205205        unsigned result = m_constantRegisters.size();
    206206        m_constantRegisters.append(WriteBarrier<Unknown>());
    … …  
    212212    {
    213213        VM& vm = *this->vm();
    214         auto locker = lockDuringMarking(vm.heap, *this);
     214        auto locker = lockDuringMarking(vm.heap, cellLock());
    215215        unsigned result = m_constantRegisters.size();
    216216        ASSERT(result);
    … …  
    275275    {
    276276        VM& vm = *this->vm();
    277         auto locker = lockDuringMarking(vm.heap, *this);
     277        auto locker = lockDuringMarking(vm.heap, cellLock());
    278278        unsigned size = m_functionDecls.size();
    279279        m_functionDecls.append(WriteBarrier<UnlinkedFunctionExecutable>());
    … …  
    286286    {
    287287        VM& vm = *this->vm();
    288         auto locker = lockDuringMarking(vm.heap, *this);
     288        auto locker = lockDuringMarking(vm.heap, cellLock());
    289289        unsigned size = m_functionExprs.size();
    290290        m_functionExprs.append(WriteBarrier<UnlinkedFunctionExecutable>());
    … …  
    416416    {
    417417        if (!m_rareData) {
    418             auto locker = lockDuringMarking(*heap(), *this);
     418            auto locker = lockDuringMarking(*heap(), cellLock());
    419419            m_rareData = std::make_unique<RareData>();
    420420        }

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

    r225768 r227906  
    118118    std::unique_ptr<Vector<StackFrame>> stackTrace = getStackTrace(exec, vm, this, useCurrentFrame);
    119119    {
    120         auto locker = holdLock(*this);
     120        auto locker = holdLock(cellLock());
    121121        m_stackTrace = WTFMove(stackTrace);
    122122    }
    … …  
    210210    addErrorInfo(vm, m_stackTrace.get(), this);
    211211    {
    212         auto locker = holdLock(*this);
     212        auto locker = holdLock(cellLock());
    213213        m_stackTrace = nullptr;
    214214    }
    … …  
    235235
    236236    {
    237         auto locker = holdLock(*thisObject);
     237        auto locker = holdLock(thisObject->cellLock());
    238238        if (thisObject->m_stackTrace) {
    239239            for (StackFrame& frame : *thisObject->m_stackTrace)

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

    r227617 r227906  
    803803   
    804804    DisallowGC disallowGC;
    805     auto locker = holdLock(*this);
     805    auto locker = holdLock(cellLock());
    806806   
    807807    if (startIndex + count > vectorLength)
    … …  
    10061006    // a weird state: some parts of it will be left uninitialized, which we will fill in here.
    10071007    DeferGC deferGC(vm.heap);
    1008     auto locker = holdLock(*this);
     1008    auto locker = holdLock(cellLock());
    10091009   
    10101010    if (moveFront && storage->m_indexBias >= count) {

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

    r226338 r227906  
    309309}
    310310
    311 void JSCell::lockSlow()
     311void JSCellLock::lockSlow()
    312312{
    313313    Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
    … …  
    315315}
    316316
    317 void JSCell::unlockSlow()
     317void JSCellLock::unlockSlow()
    318318{
    319319    Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);

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

    r225799 r227906  
    5151class PropertyNameArray;
    5252class Structure;
     53class JSCellLock;
    5354
    5455enum class GCDeferralContextArgPresense {
    … …  
    121122    // a full-blown WTF::Lock. Note that this lock is currently used in JSArray and that lock's
    122123    // ordering with the Structure lock is that the Structure lock must be acquired first.
    123     void lock();
    124     bool tryLock();
    125     void unlock();
    126     bool isLocked() const;
     124
     125    // We use this abstraction to make it easier to grep for places where we lock cells.
     126    // to lock a cell you can just do:
     127    // auto locker = holdLock(cell->cellLocker());
     128    JSCellLock& cellLock() { return *reinterpret_cast<JSCellLock*>(this); }
    127129   
    128130    JSType type() const;
    … …  
    273275private:
    274276    friend class LLIntOffsetsExtractor;
     277    friend class JSCellLock;
    275278
    276279    JS_EXPORT_PRIVATE JSObject* toObjectSlow(ExecState*, JSGlobalObject*) const;
    277     JS_EXPORT_PRIVATE void lockSlow();
    278     JS_EXPORT_PRIVATE void unlockSlow();
    279280
    280281    StructureID m_structureID;
    … …  
    285286};
    286287
     288class JSCellLock : public JSCell {
     289public:
     290    void lock();
     291    bool tryLock();
     292    void unlock();
     293    bool isLocked() const;
     294private:
     295    JS_EXPORT_PRIVATE void lockSlow();
     296    JS_EXPORT_PRIVATE void unlockSlow();
     297};
     298
    287299template<typename To, typename From>
    288300inline To jsCast(From* from)

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

    r227617 r227906  
    324324}
    325325
    326 inline void JSCell::lock()
     326inline void JSCellLock::lock()
    327327{
    328328    Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
    … …  
    331331}
    332332
    333 inline bool JSCell::tryLock()
     333inline bool JSCellLock::tryLock()
    334334{
    335335    Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
    … …  
    337337}
    338338
    339 inline void JSCell::unlock()
     339inline void JSCellLock::unlock()
    340340{
    341341    Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
    … …  
    344344}
    345345
    346 inline bool JSCell::isLocked() const
     346inline bool JSCellLock::isLocked() const
    347347{
    348348    Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);

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

    r227435 r227906  
    384384    IndexingType indexingType = structure->indexingType();
    385385    Dependency indexingTypeDependency = Dependency::fence(indexingType);
    386     Locker<JSCell> locker(NoLockingNecessary);
     386    Locker<JSCellLock> locker(NoLockingNecessary);
    387387    switch (indexingType) {
    388388    case ALL_CONTIGUOUS_INDEXING_TYPES:
    … …  
    392392        // assume that a contiguous butterfly may transform into an array storage one, though
    393393        // this is probably more conservative than necessary.
    394         locker = Locker<JSCell>(*this);
     394        locker = holdLock(cellLock());
    395395        break;
    396396    default:
    … …  
    13591359   
    13601360    if (vm.heap.mutatorShouldBeFenced()) {
    1361         auto locker = holdLock(*this);
     1361        auto locker = holdLock(cellLock());
    13621362        m_butterflyIndexingMask = newStorage->butterfly()->computeIndexingMask();
    13631363        setStructureIDDirectly(nuke(structureID()));

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

    r222017 r227906  
    7272
    7373    {
    74         auto locker = lockDuringMarking(vm.heap, *this);
     74        auto locker = lockDuringMarking(vm.heap, cellLock());
    7575        m_propertyNames.resizeToFit(vector.size());
    7676    }
    … …  
    9090    Base::visitChildren(cell, visitor);
    9191    JSPropertyNameEnumerator* thisObject = jsCast<JSPropertyNameEnumerator*>(cell);
    92     auto locker = holdLock(*thisObject);
     92    auto locker = holdLock(thisObject->cellLock());
    9393    for (auto& propertyName : thisObject->m_propertyNames)
    9494        visitor.append(propertyName);

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

    r222473 r227906  
    7878    size_t capacity;
    7979    {
    80         auto locker = holdLock(*this);
     80        auto locker = holdLock(cellLock());
    8181        SparseArrayEntry entry;
    8282        entry.setWithoutWriteBarrier(jsUndefined());
    … …  
    9696void SparseArrayValueMap::remove(iterator it)
    9797{
    98     auto locker = holdLock(*this);
     98    auto locker = holdLock(cellLock());
    9999    m_map.remove(it);
    100100}
    … …  
    102102void SparseArrayValueMap::remove(unsigned i)
    103103{
    104     auto locker = holdLock(*this);
     104    auto locker = holdLock(cellLock());
    105105    m_map.remove(i);
    106106}
    … …  
    198198{
    199199    Base::visitChildren(thisObject, visitor);
    200    
     200
     201    auto locker = holdLock(thisObject->cellLock());
    201202    SparseArrayValueMap* thisMap = jsCast<SparseArrayValueMap*>(thisObject);
    202     auto locker = holdLock(*thisMap);
    203203    iterator end = thisMap->m_map.end();
    204204    for (iterator it = thisMap->m_map.begin(); it != end; ++it)
Note: See TracChangeset for help on using the changeset viewer.