Changeset 289515 in webkit


Ignore:
Timestamp:
Feb 9, 2022, 5:50:04 PM (3 years ago)
Author:
ysuzuki@apple.com
Message:

Use local variable pointer for concurrently load value
​https://bugs.webkit.org/show_bug.cgi?id=236387

Reviewed by Saam Barati.

Consistently using local pointer to load member fields in

  1. WriteBarrierStructureID
  2. JSString's fiber
  3. Weak
  4. WriteBarrier<SomeKindOfCell>

to encourage compilers not to load the field twice.

  • heap/Weak.cpp:

(JSC::weakClearSlowCase):

  • heap/Weak.h:

(JSC::Weak::isHashTableEmptyValue const):
(JSC::Weak::unsafeImpl const):
(JSC::Weak::clear):
(JSC::Weak::impl const):

  • heap/WeakInlines.h:

(JSC::Weak<T>::isHashTableDeletedValue const):
(JSC:: const):
(JSC::Weak<T>::operator const):
(JSC::Weak<T>::get const):
(JSC::Weak<T>::leakImpl):

  • runtime/JSString.cpp:

(JSC::JSString::dumpToStream):
(JSC::JSString::estimatedSize):
(JSC::JSString::visitChildrenImpl):

  • runtime/JSString.h:

(JSC::JSString::fiberConcurrently const):
(JSC::JSString::is8Bit const):
(JSC::JSString::length const):
(JSC::JSString::tryGetValueImpl const):
(JSC::JSString::isSubstring const):

  • runtime/WriteBarrier.h:

(JSC::WriteBarrierBase::get const):
(JSC::WriteBarrierBase::operator* const):
(JSC::WriteBarrierBase::operator-> const):
(JSC::WriteBarrierBase::operator bool const):
(JSC::WriteBarrierBase::operator! const):
(JSC::WriteBarrierBase::unvalidatedGet const):
(JSC::WriteBarrierBase::cell const):
(JSC::WriteBarrierStructureID::get const):
(JSC::WriteBarrierStructureID::operator* const):
(JSC::WriteBarrierStructureID::operator-> const):
(JSC::WriteBarrierStructureID::operator bool const):
(JSC::WriteBarrierStructureID::operator! const):
(JSC::WriteBarrierStructureID::unvalidatedGet const):

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r289469 r289515  
     12022-02-09  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        Use local variable pointer for concurrently load value
     4        https://bugs.webkit.org/show_bug.cgi?id=236387
     5
     6        Reviewed by Saam Barati.
     7
     8        Consistently using local pointer to load member fields in
     9
     10            1. WriteBarrierStructureID
     11            2. JSString's fiber
     12            3. Weak
     13            4. WriteBarrier<SomeKindOfCell>
     14
     15        to encourage compilers not to load the field twice.
     16
     17        * heap/Weak.cpp:
     18        (JSC::weakClearSlowCase):
     19        * heap/Weak.h:
     20        (JSC::Weak::isHashTableEmptyValue const):
     21        (JSC::Weak::unsafeImpl const):
     22        (JSC::Weak::clear):
     23        (JSC::Weak::impl const):
     24        * heap/WeakInlines.h:
     25        (JSC::Weak<T>::isHashTableDeletedValue const):
     26        (JSC:: const):
     27        (JSC::Weak<T>::operator const):
     28        (JSC::Weak<T>::get const):
     29        (JSC::Weak<T>::leakImpl):
     30        * runtime/JSString.cpp:
     31        (JSC::JSString::dumpToStream):
     32        (JSC::JSString::estimatedSize):
     33        (JSC::JSString::visitChildrenImpl):
     34        * runtime/JSString.h:
     35        (JSC::JSString::fiberConcurrently const):
     36        (JSC::JSString::is8Bit const):
     37        (JSC::JSString::length const):
     38        (JSC::JSString::tryGetValueImpl const):
     39        (JSC::JSString::isSubstring const):
     40        * runtime/WriteBarrier.h:
     41        (JSC::WriteBarrierBase::get const):
     42        (JSC::WriteBarrierBase::operator* const):
     43        (JSC::WriteBarrierBase::operator-> const):
     44        (JSC::WriteBarrierBase::operator bool const):
     45        (JSC::WriteBarrierBase::operator! const):
     46        (JSC::WriteBarrierBase::unvalidatedGet const):
     47        (JSC::WriteBarrierBase::cell const):
     48        (JSC::WriteBarrierStructureID::get const):
     49        (JSC::WriteBarrierStructureID::operator* const):
     50        (JSC::WriteBarrierStructureID::operator-> const):
     51        (JSC::WriteBarrierStructureID::operator bool const):
     52        (JSC::WriteBarrierStructureID::operator! const):
     53        (JSC::WriteBarrierStructureID::unvalidatedGet const):
     54
    1552022-02-09  Lauro Moura  <lmoura@igalia.com>
    256

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

    r261755 r289515  
    3131namespace JSC {
    3232
    33 void weakClearSlowCase(WeakImpl*& impl)
     33void weakClearSlowCase(WeakImpl* impl)
    3434{
    3535    ASSERT(impl);
    36 
    3736    WeakSet::deallocate(impl);
    38     impl = nullptr;
    3937}
    4038

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

    r256779 r289515  
    2828#include "JSExportMacros.h"
    2929#include <cstddef>
     30#include <wtf/Atomics.h>
    3031#include <wtf/HashTraits.h>
    3132#include <wtf/Noncopyable.h>
    … …  
    3839
    3940// This is a free function rather than a Weak<T> member function so we can put it in Weak.cpp.
    40 JS_EXPORT_PRIVATE void weakClearSlowCase(WeakImpl*&);
     41JS_EXPORT_PRIVATE void weakClearSlowCase(WeakImpl*);
    4142
    4243template<typename T> class Weak {
    … …  
    5455    bool isHashTableDeletedValue() const;
    5556    Weak(WTF::HashTableDeletedValueType);
    56     constexpr bool isHashTableEmptyValue() const { return !m_impl; }
     57    constexpr bool isHashTableEmptyValue() const { return !impl(); }
    5758
    5859    Weak(Weak&&);
    … …  
    7778
    7879    inline WeakImpl* leakImpl() WARN_UNUSED_RETURN;
    79     WeakImpl* unsafeImpl() const { return m_impl; }
     80    WeakImpl* unsafeImpl() const { return impl(); }
    8081    void clear()
    8182    {
    82         if (!m_impl)
     83        auto* pointer = impl();
     84        if (!pointer)
    8385            return;
    84         weakClearSlowCase(m_impl);
     86        weakClearSlowCase(pointer);
     87        m_impl = nullptr;
    8588    }
    8689   
    8790private:
    8891    static inline WeakImpl* hashTableDeletedValue();
     92    WeakImpl* impl() const { return m_impl; }
    8993
    9094    WeakImpl* m_impl { nullptr };

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

    r261464 r289515  
    3939template<typename T> inline bool Weak<T>::isHashTableDeletedValue() const
    4040{
    41     return m_impl == hashTableDeletedValue();
     41    return impl() == hashTableDeletedValue();
    4242}
    4343
    … …  
    7171template<typename T> inline T* Weak<T>::operator->() const
    7272{
    73     ASSERT(m_impl && m_impl->state() == WeakImpl::Live);
     73    auto* pointer = impl();
     74    ASSERT(pointer && pointer->state() == WeakImpl::Live);
    7475    // We can't use jsCast here since we could be called in a finalizer.
    75     return static_cast<T*>(m_impl->jsValue().asCell());
     76    return static_cast<T*>(pointer->jsValue().asCell());
    7677}
    7778
    7879template<typename T> inline T& Weak<T>::operator*() const
    7980{
    80     ASSERT(m_impl && m_impl->state() == WeakImpl::Live);
     81    auto* pointer = impl();
     82    ASSERT(pointer && pointer->state() == WeakImpl::Live);
    8183    // We can't use jsCast here since we could be called in a finalizer.
    82     return *static_cast<T*>(m_impl->jsValue().asCell());
     84    return *static_cast<T*>(pointer->jsValue().asCell());
    8385}
    8486
    8587template<typename T> inline T* Weak<T>::get() const
    8688{
    87     if (!m_impl || m_impl->state() != WeakImpl::Live)
     89    auto* pointer = impl();
     90    if (!pointer || pointer->state() != WeakImpl::Live)
    8891        return nullptr;
    8992    // We can't use jsCast here since we could be called in a finalizer.
    90     return static_cast<T*>(m_impl->jsValue().asCell());
     93    return static_cast<T*>(pointer->jsValue().asCell());
    9194}
    9295
    … …  
    98101template<typename T> inline bool Weak<T>::operator!() const
    99102{
    100     return !m_impl || !m_impl->jsValue() || m_impl->state() != WeakImpl::Live;
     103    auto* pointer = impl();
     104    return !pointer || !pointer->jsValue() || pointer->state() != WeakImpl::Live;
    101105}
    102106
    … …  
    108112template<typename T> inline WeakImpl* Weak<T>::leakImpl()
    109113{
    110     WeakImpl* impl = m_impl;
     114    auto* pointer = impl();
    111115    m_impl = nullptr;
    112     return impl;
     116    return pointer;
    113117}
    114118

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

    r288537 r289515  
    6969    const JSString* thisObject = jsCast<const JSString*>(cell);
    7070    out.printf("<%p, %s, [%u], ", thisObject, thisObject->className(vm), thisObject->length());
    71     uintptr_t pointer = thisObject->m_fiber;
     71    uintptr_t pointer = thisObject->fiberConcurrently();
    7272    if (pointer & isRopeInPointer) {
    7373        if (pointer & JSRopeString::isSubstringInPointer)
    … …  
    104104{
    105105    JSString* thisObject = asString(cell);
    106     uintptr_t pointer = thisObject->m_fiber;
     106    uintptr_t pointer = thisObject->fiberConcurrently();
    107107    if (pointer & isRopeInPointer)
    108108        return Base::estimatedSize(cell, vm);
    … …  
    117117    Base::visitChildren(thisObject, visitor);
    118118   
    119     uintptr_t pointer = thisObject->m_fiber;
     119    uintptr_t pointer = thisObject->fiberConcurrently();
    120120    if (pointer & isRopeInPointer) {
    121121        if (pointer & JSRopeString::isSubstringInPointer) {

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

    r289359 r289515  
    237237    JS_EXPORT_PRIVATE bool equalSlowCase(JSGlobalObject*, JSString* other) const;
    238238    bool isSubstring() const;
     239
     240    uintptr_t fiberConcurrently() const { return m_fiber; }
    239241
    240242    mutable uintptr_t m_fiber;
    … …  
    696698ALWAYS_INLINE bool JSString::is8Bit() const
    697699{
    698     uintptr_t pointer = m_fiber;
     700    uintptr_t pointer = fiberConcurrently();
    699701    if (pointer & isRopeInPointer) {
    700702        // Do not load m_fiber twice. We should use the information in pointer.
    … …  
    710712ALWAYS_INLINE unsigned JSString::length() const
    711713{
    712     uintptr_t pointer = m_fiber;
     714    uintptr_t pointer = fiberConcurrently();
    713715    if (pointer & isRopeInPointer)
    714716        return jsCast<const JSRopeString*>(this)->length();
    … …  
    724726inline const StringImpl* JSString::tryGetValueImpl() const
    725727{
    726     uintptr_t pointer = m_fiber;
     728    uintptr_t pointer = fiberConcurrently();
    727729    if (pointer & isRopeInPointer)
    728730        return nullptr;
    … …  
    10741076inline bool JSString::isSubstring() const
    10751077{
    1076     return m_fiber & JSRopeString::isSubstringInPointer;
     1078    return fiberConcurrently() & JSRopeString::isSubstringInPointer;
    10771079}
    10781080

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

    r288815 r289515  
    9999    {
    100100        // Copy m_cell to a local to avoid multiple-read issues. (See <http://webkit.org/b/110854>)
    101         StorageType cell = m_cell;
     101        StorageType cell = this->cell();
    102102        if (cell)
    103103            validateCell(reinterpret_cast<JSCell*>(static_cast<void*>(Traits::unwrap(cell))));
    … …  
    107107    T* operator*() const
    108108    {
    109         StorageType cell = m_cell;
     109        StorageType cell = this->cell();
    110110        ASSERT(cell);
    111111        auto unwrapped = Traits::unwrap(cell);
    … …  
    116116    T* operator->() const
    117117    {
    118         StorageType cell = m_cell;
     118        StorageType cell = this->cell();
    119119        ASSERT(cell);
    120120        auto unwrapped = Traits::unwrap(cell);
    … …  
    136136    }
    137137   
    138     explicit operator bool() const { return !!m_cell; }
    139    
    140     bool operator!() const { return !m_cell; }
     138    explicit operator bool() const { return !!cell(); }
     139   
     140    bool operator!() const { return !cell(); }
    141141
    142142    void setWithoutWriteBarrier(T* value)
    … …  
    148148    }
    149149
    150     T* unvalidatedGet() const { return Traits::unwrap(m_cell); }
     150    T* unvalidatedGet() const { return Traits::unwrap(cell()); }
    151151
    152152private:
     153    StorageType cell() const { return m_cell; }
     154
    153155    StorageType m_cell;
    154156};
    … …  
    282284    {
    283285        // Copy m_structureID to a local to avoid multiple-read issues. (See <http://webkit.org/b/110854>)
    284         StructureID structureID = m_structureID;
     286        StructureID structureID = value();
    285287        if (structureID) {
    286288            Structure* structure = structureID.decode();
    … …  
    293295    Structure* operator*() const
    294296    {
    295         StructureID structureID = m_structureID;
     297        StructureID structureID = value();
    296298        ASSERT(structureID);
    297299        Structure* structure = structureID.decode();
    … …  
    302304    Structure* operator->() const
    303305    {
    304         StructureID structureID = m_structureID;
     306        StructureID structureID = value();
    305307        ASSERT(structureID);
    306308        Structure* structure = structureID.decode();
    … …  
    316318    explicit operator bool() const
    317319    {
    318         return !!m_structureID;
     320        return !!value();
    319321    }
    320322
    321323    bool operator!() const
    322324    {
    323         return !m_structureID;
     325        return !value();
    324326    }
    325327
    … …  
    338340    Structure* unvalidatedGet() const
    339341    {
    340         StructureID structureID = m_structureID;
     342        StructureID structureID = value();
    341343        if (structureID)
    342344            return structureID.decode();
Note: See TracChangeset for help on using the changeset viewer.