Changeset 220548 in webkit


Ignore:
Timestamp:
Aug 10, 2017 2:19:51 PM (7 years ago)
Author:
Yusuke Suzuki
Message:

[WTF] ThreadSpecific should not introduce additional indirection
https://bugs.webkit.org/show_bug.cgi?id=175187

Reviewed by Mark Lam.

Source/JavaScriptCore:

  • runtime/Identifier.cpp:

Source/WebCore:

We drop ThreadSpecific::replace feature which is only used by
Web thread. Instead, we use ThreadSpecific<std::unique_ptr<T>> here.

While this std::unique_ptr<T> shares one instance between main thread
and web thread, this is the same to the current implementation. It is
safe because the web thread never finishes.

And for non-web thread implementation, we just use ThreadSpecific<T>,
since it is the most efficient.

  • platform/ThreadGlobalData.cpp:

(WebCore::ThreadGlobalData::ThreadGlobalData):
(WebCore::ThreadGlobalData::setWebCoreThreadData):
(WebCore::threadGlobalData):
We also drop StringImpl::empty() call since it is not necessary now:
StringImpl::empty data is statically initialized by using constexpr.

  • platform/ThreadGlobalData.h:

We make it FAST_ALLOCATED since it is previously allocated by fast malloc
in ThreadSpecific.

Source/WTF:

ThreadSpecific sets Data* to the TLS. And Data holds T*, which
is fast allocated actual data. But ideally, we should store T
instance directly in Data instead of introducing an additional
indirection.

This patch adds storage in Data in order to embed the instance of T. The constructor
for Data will invoke the constructor for T on the embedded storage. We also drop
ThreadSpecific::replace which is only used by the web thread to set its thread specific
ThreadGlobalData to the one shared from the main thread. The existing implementation
relies on the main thread and the web thread never exiting in order for the shared
ThreadGlobalData to stay alive. We can achieve the same semantics by using a
ThreadSpecific<std::unique_ptr<T>> to hold the ThreadGlobalData instance instead.

  • wtf/ThreadSpecific.h:

(WTF::ThreadSpecific::Data::construct):
(WTF::ThreadSpecific::Data::Data):
We make it fast allocated since we previously allocated ThreadSpecific T data by fastMalloc.

(WTF::ThreadSpecific::Data::~Data):
(WTF::ThreadSpecific::Data::storagePointer const):
(WTF::canBeGCThread>::get):
We also drop RELEASE_ASSERT from ::get(). We already inserted this assert to setAndConstruct(),
so when creating the member to this TLS, we execute this release assert. So it is
not necessary to execute this assertion every time we get data from this TLS.

(WTF::canBeGCThread>::set):
(WTF::canBeGCThread>::destroy):
(WTF::canBeGCThread>::setAndConstruct):
(WTF::T):
(WTF::canBeGCThread>::replace): Deleted.

Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r220538 r220548  
     12017-08-09  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] ThreadSpecific should not introduce additional indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=175187
     5
     6        Reviewed by Mark Lam.
     7
     8        * runtime/Identifier.cpp:
     9
    1102017-08-10  Tim Horton  <timothy_horton@apple.com>
    211
  • trunk/Source/JavaScriptCore/runtime/Identifier.cpp

    r220186 r220548  
    3333#include <wtf/text/ASCIIFastPath.h>
    3434#include <wtf/text/StringHash.h>
    35 
    36 using WTF::ThreadSpecific;
    3735
    3836namespace JSC {
  • trunk/Source/WTF/ChangeLog

    r220541 r220548  
     12017-08-09  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] ThreadSpecific should not introduce additional indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=175187
     5
     6        Reviewed by Mark Lam.
     7
     8        ThreadSpecific sets Data* to the TLS. And Data holds T*, which
     9        is fast allocated actual data. But ideally, we should store T
     10        instance directly in Data instead of introducing an additional
     11        indirection.
     12
     13        This patch adds storage in Data in order to embed the instance of T. The constructor
     14        for Data will invoke the constructor for T on the embedded storage. We also drop
     15        ThreadSpecific::replace which is only used by the web thread to set its thread specific
     16        ThreadGlobalData to the one shared from the main thread. The existing implementation
     17        relies on the main thread and the web thread never exiting in order for the shared
     18        ThreadGlobalData to stay alive. We can achieve the same semantics by using a
     19        ThreadSpecific<std::unique_ptr<T>> to hold the ThreadGlobalData instance instead.
     20
     21        * wtf/ThreadSpecific.h:
     22        (WTF::ThreadSpecific::Data::construct):
     23        (WTF::ThreadSpecific::Data::Data):
     24        We make it fast allocated since we previously allocated ThreadSpecific T data by fastMalloc.
     25
     26        (WTF::ThreadSpecific::Data::~Data):
     27        (WTF::ThreadSpecific::Data::storagePointer const):
     28        (WTF::canBeGCThread>::get):
     29        We also drop RELEASE_ASSERT from ::get(). We already inserted this assert to setAndConstruct(),
     30        so when creating the member to this TLS, we execute this release assert. So it is
     31        not necessary to execute this assertion every time we get data from this TLS.
     32
     33        (WTF::canBeGCThread>::set):
     34        (WTF::canBeGCThread>::destroy):
     35        (WTF::canBeGCThread>::setAndConstruct):
     36        (WTF::T):
     37        (WTF::canBeGCThread>::replace): Deleted.
     38
    1392017-08-10  Tim Horton  <timothy_horton@apple.com>
    240
  • trunk/Source/WTF/wtf/ThreadSpecific.h

    r215265 r220548  
    8383    T& operator*();
    8484
    85 #if USE(WEB_THREAD)
    86     void replace(T*);
    87 #endif
    88 
    8985private:
    9086    // Not implemented. It's technically possible to destroy a thread specific key, but one would need
     
    9490    ~ThreadSpecific();
    9591
    96     T* get();
    97     void set(T*);
    98     void static THREAD_SPECIFIC_CALL destroy(void* ptr);
    99 
    10092    struct Data {
    10193        WTF_MAKE_NONCOPYABLE(Data);
     94        WTF_MAKE_FAST_ALLOCATED;
    10295    public:
    103         Data(T* value, ThreadSpecific<T, canBeGCThread>* owner) : value(value), owner(owner) {}
    104 
    105         T* value;
     96        using PointerType = typename std::remove_const<T>::type*;
     97
     98        Data(ThreadSpecific<T, canBeGCThread>* owner)
     99            : owner(owner)
     100        {
     101            // Set up thread-specific value's memory pointer before invoking constructor, in case any function it calls
     102            // needs to access the value, to avoid recursion.
     103            owner->setInTLS(this);
     104            new (NotNull, storagePointer()) T;
     105        }
     106
     107        ~Data()
     108        {
     109            storagePointer()->~T();
     110            owner->setInTLS(nullptr);
     111        }
     112
     113        PointerType storagePointer() const { return const_cast<PointerType>(reinterpret_cast<const T*>(&m_storage)); }
     114
     115        typename std::aligned_storage<sizeof(T), std::alignment_of<T>::value>::type m_storage;
    106116        ThreadSpecific<T, canBeGCThread>* owner;
    107117    };
     118
     119    T* get();
     120    T* set();
     121    void setInTLS(Data*);
     122    void static THREAD_SPECIFIC_CALL destroy(void* ptr);
    108123
    109124#if USE(PTHREADS)
     
    157172    Data* data = static_cast<Data*>(pthread_getspecific(m_key));
    158173    if (data)
    159         return data->value;
    160     RELEASE_ASSERT(canBeGCThread == CanBeGCThread::True || !mayBeGCThread());
     174        return data->storagePointer();
    161175    return nullptr;
    162176}
    163177
    164178template<typename T, CanBeGCThread canBeGCThread>
    165 inline void ThreadSpecific<T, canBeGCThread>::set(T* ptr)
    166 {
    167     RELEASE_ASSERT(canBeGCThread == CanBeGCThread::True || !mayBeGCThread());
    168     ASSERT(!get());
    169     pthread_setspecific(m_key, new Data(ptr, this));
     179inline void ThreadSpecific<T, canBeGCThread>::setInTLS(Data* data)
     180{
     181    pthread_setspecific(m_key, data);
    170182}
    171183
     
    233245    Data* data = static_cast<Data*>(FlsGetValue(flsKeys()[m_index]));
    234246    if (data)
    235         return data->value;
    236     RELEASE_ASSERT(canBeGCThread == CanBeGCThread::True || !mayBeGCThread());
     247        return data->storagePointer();
    237248    return nullptr;
    238249}
    239250
    240251template<typename T, CanBeGCThread canBeGCThread>
    241 inline void ThreadSpecific<T, canBeGCThread>::set(T* ptr)
    242 {
    243     RELEASE_ASSERT(canBeGCThread == CanBeGCThread::True || !mayBeGCThread());
    244     ASSERT(!get());
    245     Data* data = new Data(ptr, this);
     252inline void ThreadSpecific<T, canBeGCThread>::setInTLS(Data* data)
     253{
    246254    FlsSetValue(flsKeys()[m_index], data);
    247255}
     
    262270#endif
    263271
    264     data->value->~T();
    265     fastFree(data->value);
    266 
    267 #if USE(PTHREADS)
    268     pthread_setspecific(data->owner->m_key, 0);
    269 #elif OS(WINDOWS)
    270     FlsSetValue(flsKeys()[data->owner->m_index], 0);
    271 #else
    272 #error ThreadSpecific is not implemented for this platform.
    273 #endif
    274 
    275272    delete data;
    276273}
    277274
    278275template<typename T, CanBeGCThread canBeGCThread>
     276inline T* ThreadSpecific<T, canBeGCThread>::set()
     277{
     278    RELEASE_ASSERT(canBeGCThread == CanBeGCThread::True || !mayBeGCThread());
     279    ASSERT(!get());
     280    Data* data = new Data(this); // Data will set itself into TLS.
     281    ASSERT(get() == data->storagePointer());
     282    return data->storagePointer();
     283}
     284
     285template<typename T, CanBeGCThread canBeGCThread>
    279286inline bool ThreadSpecific<T, canBeGCThread>::isSet()
    280287{
     
    285292inline ThreadSpecific<T, canBeGCThread>::operator T*()
    286293{
    287     T* ptr = static_cast<T*>(get());
    288     if (!ptr) {
    289         // Set up thread-specific value's memory pointer before invoking constructor, in case any function it calls
    290         // needs to access the value, to avoid recursion.
    291         ptr = static_cast<T*>(fastZeroedMalloc(sizeof(T)));
    292         set(ptr);
    293         new (NotNull, ptr) T;
    294     }
    295     return ptr;
     294    if (T* ptr = get())
     295        return ptr;
     296    return set();
    296297}
    297298
     
    308309}
    309310
    310 #if USE(WEB_THREAD)
    311 template<typename T, CanBeGCThread canBeGCThread>
    312 inline void ThreadSpecific<T, canBeGCThread>::replace(T* newPtr)
    313 {
    314     ASSERT(newPtr);
    315     Data* data = static_cast<Data*>(pthread_getspecific(m_key));
    316     ASSERT(data);
    317     data->value->~T();
    318     fastFree(data->value);
    319     data->value = newPtr;
    320 }
    321 #endif
    322 
    323311} // namespace WTF
    324312
     313using WTF::ThreadSpecific;
     314
    325315#endif // WTF_ThreadSpecific_h
  • trunk/Source/WebCore/ChangeLog

    r220540 r220548  
     12017-08-09  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] ThreadSpecific should not introduce additional indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=175187
     5
     6        Reviewed by Mark Lam.
     7
     8        We drop ThreadSpecific::replace feature which is only used by
     9        Web thread. Instead, we use ThreadSpecific<std::unique_ptr<T>> here.
     10
     11        While this std::unique_ptr<T> shares one instance between main thread
     12        and web thread, this is the same to the current implementation. It is
     13        safe because the web thread never finishes.
     14
     15        And for non-web thread implementation, we just use ThreadSpecific<T>,
     16        since it is the most efficient.
     17
     18        * platform/ThreadGlobalData.cpp:
     19        (WebCore::ThreadGlobalData::ThreadGlobalData):
     20        (WebCore::ThreadGlobalData::setWebCoreThreadData):
     21        (WebCore::threadGlobalData):
     22        We also drop StringImpl::empty() call since it is not necessary now:
     23        StringImpl::empty data is statically initialized by using constexpr.
     24
     25        * platform/ThreadGlobalData.h:
     26        We make it FAST_ALLOCATED since it is previously allocated by fast malloc
     27        in ThreadSpecific.
     28
    1292017-08-10  Michael Catanzaro  <mcatanzaro@igalia.com>
    230
  • trunk/Source/WebCore/platform/ThreadGlobalData.cpp

    r220186 r220548  
    4444namespace WebCore {
    4545
    46 ThreadSpecific<ThreadGlobalData>* ThreadGlobalData::staticData;
    47 #if USE(WEB_THREAD)
    48 ThreadGlobalData* ThreadGlobalData::sharedMainThreadStaticData;
    49 #endif
    50 
    5146ThreadGlobalData::ThreadGlobalData()
    5247    : m_cachedResourceRequestInitiators(std::make_unique<CachedResourceRequestInitiators>())
     
    6762    // threadsafe.
    6863    Thread::current();
    69     StringImpl::empty();
    7064}
    7165
     
    8882
    8983#if USE(WEB_THREAD)
     84static ThreadSpecific<std::unique_ptr<ThreadGlobalData>>* staticData { nullptr };
     85static ThreadGlobalData* sharedMainThreadStaticData { nullptr };
     86
    9087void ThreadGlobalData::setWebCoreThreadData()
    9188{
    9289    ASSERT(isWebThread());
    93     ASSERT(&threadGlobalData() != ThreadGlobalData::sharedMainThreadStaticData);
     90    ASSERT(&threadGlobalData() != sharedMainThreadStaticData);
    9491
    9592    // Set WebThread's ThreadGlobalData object to be the same as the main UI thread.
    96     ThreadGlobalData::staticData->replace(ThreadGlobalData::sharedMainThreadStaticData);
     93    // The web thread never finishes, and we expect the main thread to also never finish.
     94    // Hence, it is safe to store the same ThreadGlobalData pointer in a thread specific std::unique_ptr.
     95    // FIXME: Make ThreadGlobalData RefCounted for web thread.
     96    // https://bugs.webkit.org/show_bug.cgi?id=175439
     97    (**staticData).reset(sharedMainThreadStaticData);
    9798
    98     ASSERT(&threadGlobalData() == ThreadGlobalData::sharedMainThreadStaticData);
     99    ASSERT(&threadGlobalData() == sharedMainThreadStaticData);
    99100}
    100 #endif
    101101
    102 ThreadGlobalData& threadGlobalData() 
     102ThreadGlobalData& threadGlobalData()
    103103{
    104 #if USE(WEB_THREAD)
    105     if (UNLIKELY(!ThreadGlobalData::staticData)) {
    106         ThreadGlobalData::staticData = new ThreadSpecific<ThreadGlobalData>;
     104    if (UNLIKELY(!staticData)) {
     105        staticData = new ThreadSpecific<std::unique_ptr<ThreadGlobalData>>;
     106        auto& result = **staticData;
     107        ASSERT(!result);
     108        result.reset(new ThreadGlobalData());
    107109        // WebThread and main UI thread need to share the same object. Save it in a static
    108110        // here, the WebThread will pick it up in setWebCoreThreadData().
    109111        if (pthread_main_np())
    110             ThreadGlobalData::sharedMainThreadStaticData = *ThreadGlobalData::staticData;
     112            sharedMainThreadStaticData = result.get();
     113        return *result;
    111114    }
    112     return **ThreadGlobalData::staticData;
    113 #else
    114     if (!ThreadGlobalData::staticData)
    115         ThreadGlobalData::staticData = new ThreadSpecific<ThreadGlobalData>;
    116     return **ThreadGlobalData::staticData;
    117 #endif
     115
     116    auto& result = **staticData;
     117    if (!result)
     118        result.reset(new ThreadGlobalData());
     119    return *result;
    118120}
    119121
     122#else
     123
     124static ThreadSpecific<ThreadGlobalData>* staticData { nullptr };
     125
     126ThreadGlobalData& threadGlobalData()
     127{
     128    if (UNLIKELY(!staticData))
     129        staticData = new ThreadSpecific<ThreadGlobalData>;
     130    return **staticData;
     131}
     132
     133#endif
     134
    120135} // namespace WebCore
  • trunk/Source/WebCore/platform/ThreadGlobalData.h

    r218799 r220548  
    3030#include <wtf/text/StringHash.h>
    3131
    32 #include <wtf/ThreadSpecific.h>
    33 using WTF::ThreadSpecific;
    34 
    3532namespace WebCore {
    3633
     
    4542    class ThreadGlobalData {
    4643        WTF_MAKE_NONCOPYABLE(ThreadGlobalData);
     44        WTF_MAKE_FAST_ALLOCATED;
    4745    public:
    4846        WEBCORE_EXPORT ThreadGlobalData();
     
    8179#endif
    8280
    83         WEBCORE_EXPORT static ThreadSpecific<ThreadGlobalData>* staticData;
    84 #if USE(WEB_THREAD)
    85         WEBCORE_EXPORT static ThreadGlobalData* sharedMainThreadStaticData;
    86 #endif
    8781        WEBCORE_EXPORT friend ThreadGlobalData& threadGlobalData();
    8882    };
Note: See TracChangeset for help on using the changeset viewer.