Changeset 183212 in webkit


Ignore:
Timestamp:
Apr 23, 2015 2:56:23 PM (9 years ago)
Author:
commit-queue@webkit.org
Message:

Make FunctionRareData allocation thread-safe
​https://bugs.webkit.org/show_bug.cgi?id=144001

Patch by Basile Clement <​basile_clement@apple.com> on 2015-04-23
Reviewed by Mark Lam.

The two things we want to prevent are:

  1. A thread seeing a pointer to a not-yet-fully-created rare data from a JSFunction
  2. A thread seeing a pointer to a not-yet-fully-created Structure from an ObjectAllocationProfile

For 1., only the JS thread can be creating the rare data (in
runtime/CommonSlowPaths.cpp or in dfg/DFGOperations.cpp), so we don't need to
worry about concurrent writes, and we don't need any fences when *reading* the
rare data from the JS thread. Thus we only need a storeStoreFence between the
rare data creation and assignment to m_rareData in
JSFunction::createAndInitializeRareData() to ensure that when the store to
m_rareData is issued, the rare data has been properly created.

For the DFG compilation threads, the only place they can access the
rare data is through JSFunction::rareData(), and so we only need a
loadLoadFence there to ensure that when we see a non-null pointer in
m_rareData, the pointed object will be seen as a fully created
FunctionRareData.

For 2., the structure is created in
ObjectAllocationProfile::initialize() (which appears to be called only by the
JS thread as well, in bytecode/CodeBlock.cpp and on rare data initialization,
which always happen in the JS thread), and read through
ObjectAllocationProfile::structure() and
ObjectAllocationProfile::inlineCapacity(), so following the same reasoning we
put a storeStoreFence in ObjectAllocationProfile::initialize() and a
loadLoadFence in ObjectAllocationProfile::structure() (and change
ObjectAllocationProfile::inlineCapacity() to go through
ObjectAllocationProfile::structure()).

We don't need a fence in ObjectAllocationProfile::clear() because
clearing the structure is already as atomic as it gets.

Finally, notice that we don't care about the ObjectAllocationProfile's
m_allocator as that is only used by ObjectAllocationProfile::initialize() and
ObjectAllocationProfile::clear() that are always run in the JS thread.
ObjectAllocationProfile::isNull() could cause some trouble, but it is
currently only used in the ObjectAllocationProfile::clear()'s ASSERT in the JS
thread. Doing isNull()-style pre-checks would be wrong in any other concurrent
thread anyway.

  • bytecode/ObjectAllocationProfile.h:

(JSC::ObjectAllocationProfile::initialize):
(JSC::ObjectAllocationProfile::structure):
(JSC::ObjectAllocationProfile::inlineCapacity):

  • runtime/JSFunction.cpp:

(JSC::JSFunction::allocateAndInitializeRareData):

  • runtime/JSFunction.h:

(JSC::JSFunction::rareData):
(JSC::JSFunction::allocationStructure): Deleted.
This is no longer used, as all the accesses to the ObjectAllocationProfile go through the rare data.

Location:
trunk/Source/JavaScriptCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r183207 r183212  
     12015-04-23  Basile Clement  <basile_clement@apple.com>
     2
     3        Make FunctionRareData allocation thread-safe
     4        https://bugs.webkit.org/show_bug.cgi?id=144001
     5
     6        Reviewed by Mark Lam.
     7
     8        The two things we want to prevent are:
     9
     10         1. A thread seeing a pointer to a not-yet-fully-created rare data from
     11            a JSFunction
     12         2. A thread seeing a pointer to a not-yet-fully-created Structure from
     13            an ObjectAllocationProfile
     14
     15        For 1., only the JS thread can be creating the rare data (in
     16        runtime/CommonSlowPaths.cpp or in dfg/DFGOperations.cpp), so we don't need to
     17        worry about concurrent writes, and we don't need any fences when *reading* the
     18        rare data from the JS thread. Thus we only need a storeStoreFence between the
     19        rare data creation and assignment to m_rareData in
     20        JSFunction::createAndInitializeRareData() to ensure that when the store to
     21        m_rareData is issued, the rare data has been properly created.
     22
     23        For the DFG compilation threads, the only place they can access the
     24        rare data is through JSFunction::rareData(), and so we only need a
     25        loadLoadFence there to ensure that when we see a non-null pointer in
     26        m_rareData, the pointed object will be seen as a fully created
     27        FunctionRareData.
     28
     29
     30        For 2., the structure is created in
     31        ObjectAllocationProfile::initialize() (which appears to be called only by the
     32        JS thread as well, in bytecode/CodeBlock.cpp and on rare data initialization,
     33        which always happen in the JS thread), and read through
     34        ObjectAllocationProfile::structure() and
     35        ObjectAllocationProfile::inlineCapacity(), so following the same reasoning we
     36        put a storeStoreFence in ObjectAllocationProfile::initialize() and a
     37        loadLoadFence in ObjectAllocationProfile::structure() (and change
     38        ObjectAllocationProfile::inlineCapacity() to go through
     39        ObjectAllocationProfile::structure()).
     40
     41        We don't need a fence in ObjectAllocationProfile::clear() because
     42        clearing the structure is already as atomic as it gets.
     43
     44        Finally, notice that we don't care about the ObjectAllocationProfile's
     45        m_allocator as that is only used by ObjectAllocationProfile::initialize() and
     46        ObjectAllocationProfile::clear() that are always run in the JS thread.
     47        ObjectAllocationProfile::isNull() could cause some trouble, but it is
     48        currently only used in the ObjectAllocationProfile::clear()'s ASSERT in the JS
     49        thread.  Doing isNull()-style pre-checks would be wrong in any other concurrent
     50        thread anyway.
     51
     52        * bytecode/ObjectAllocationProfile.h:
     53        (JSC::ObjectAllocationProfile::initialize):
     54        (JSC::ObjectAllocationProfile::structure):
     55        (JSC::ObjectAllocationProfile::inlineCapacity):
     56        * runtime/JSFunction.cpp:
     57        (JSC::JSFunction::allocateAndInitializeRareData):
     58        * runtime/JSFunction.h:
     59        (JSC::JSFunction::rareData):
     60        (JSC::JSFunction::allocationStructure): Deleted.
     61        This is no longer used, as all the accesses to the ObjectAllocationProfile go through the rare data.
     62
    1632015-04-22  Filip Pizlo  <fpizlo@apple.com>
    264

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

    r182280 r183212  
    9090            inlineCapacity = JSFinalObject::maxInlineCapacity();
    9191
     92        Structure* structure = vm.prototypeMap.emptyObjectStructureForPrototype(prototype, inlineCapacity);
     93
     94        // Ensure that if another thread sees the structure, it will see it properly created
     95        WTF::storeStoreFence();
     96
    9297        m_allocator = allocator;
    93         m_structure.set(vm, owner,
    94             vm.prototypeMap.emptyObjectStructureForPrototype(prototype, inlineCapacity));
     98        m_structure.set(vm, owner, structure);
    9599    }
    96100
    97     Structure* structure() { return m_structure.get(); }
    98     unsigned inlineCapacity() { return m_structure->inlineCapacity(); }
     101    Structure* structure()
     102    {
     103        Structure* structure = m_structure.get();
     104        // Ensure that if we see the structure, it has been properly created
     105        WTF::loadLoadFence();
     106        return structure;
     107    }
     108    unsigned inlineCapacity() { return structure()->inlineCapacity(); }
    99109
    100110    void clear()

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

    r183113 r183212  
    117117        prototype = globalObject()->objectPrototype();
    118118    FunctionRareData* rareData = FunctionRareData::create(vm, prototype, inlineCapacity);
     119
     120    // A DFG compilation thread may be trying to read the rare data
     121    // We want to ensure that it sees it properly allocated
     122    WTF::storeStoreFence();
     123
    119124    m_rareData.set(vm, this, rareData);
    120125    return m_rareData.get();

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

    r183113 r183212  
    119119    }
    120120
    121     FunctionRareData* rareData() { return m_rareData.get(); }
     121    FunctionRareData* rareData()
     122    {
     123        FunctionRareData* rareData = m_rareData.get();
    122124
    123     Structure* allocationStructure()
    124     {
    125         if (!m_rareData)
    126             return nullptr;
     125        // The JS thread may be concurrently creating the rare data
     126        // If we see it, we want to ensure it has been properly created
     127        WTF::loadLoadFence();
    127128
    128         return m_rareData.get()->allocationStructure();
     129        return rareData;
    129130    }
    130131
Note: See TracChangeset for help on using the changeset viewer.