Title: [183212] trunk/Source/_javascript_Core
- Revision
- 183212
- Author
- [email protected]
- Date
- 2015-04-23 14:56:23 -0700 (Thu, 23 Apr 2015)
Log Message
Make FunctionRareData allocation thread-safe
https://bugs.webkit.org/show_bug.cgi?id=144001
Patch by Basile Clement <[email protected]> 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.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (183211 => 183212)
--- trunk/Source/_javascript_Core/ChangeLog 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-04-23 21:56:23 UTC (rev 183212)
@@ -1,3 +1,65 @@
+2015-04-23 Basile Clement <[email protected]>
+
+ Make FunctionRareData allocation thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=144001
+
+ 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.
+
2015-04-22 Filip Pizlo <[email protected]>
DFG should insert Phantoms late using BytecodeKills and block-local OSR availability
Modified: trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfile.h (183211 => 183212)
--- trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfile.h 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfile.h 2015-04-23 21:56:23 UTC (rev 183212)
@@ -89,13 +89,23 @@
if (inlineCapacity > JSFinalObject::maxInlineCapacity())
inlineCapacity = JSFinalObject::maxInlineCapacity();
+ Structure* structure = vm.prototypeMap.emptyObjectStructureForPrototype(prototype, inlineCapacity);
+
+ // Ensure that if another thread sees the structure, it will see it properly created
+ WTF::storeStoreFence();
+
m_allocator = allocator;
- m_structure.set(vm, owner,
- vm.prototypeMap.emptyObjectStructureForPrototype(prototype, inlineCapacity));
+ m_structure.set(vm, owner, structure);
}
- Structure* structure() { return m_structure.get(); }
- unsigned inlineCapacity() { return m_structure->inlineCapacity(); }
+ Structure* structure()
+ {
+ Structure* structure = m_structure.get();
+ // Ensure that if we see the structure, it has been properly created
+ WTF::loadLoadFence();
+ return structure;
+ }
+ unsigned inlineCapacity() { return structure()->inlineCapacity(); }
void clear()
{
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (183211 => 183212)
--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2015-04-23 21:56:23 UTC (rev 183212)
@@ -116,6 +116,11 @@
if (!prototype)
prototype = globalObject()->objectPrototype();
FunctionRareData* rareData = FunctionRareData::create(vm, prototype, inlineCapacity);
+
+ // A DFG compilation thread may be trying to read the rare data
+ // We want to ensure that it sees it properly allocated
+ WTF::storeStoreFence();
+
m_rareData.set(vm, this, rareData);
return m_rareData.get();
}
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.h (183211 => 183212)
--- trunk/Source/_javascript_Core/runtime/JSFunction.h 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.h 2015-04-23 21:56:23 UTC (rev 183212)
@@ -118,14 +118,15 @@
return m_rareData.get();
}
- FunctionRareData* rareData() { return m_rareData.get(); }
-
- Structure* allocationStructure()
+ FunctionRareData* rareData()
{
- if (!m_rareData)
- return nullptr;
+ FunctionRareData* rareData = m_rareData.get();
- return m_rareData.get()->allocationStructure();
+ // The JS thread may be concurrently creating the rare data
+ // If we see it, we want to ensure it has been properly created
+ WTF::loadLoadFence();
+
+ return rareData;
}
bool isHostOrBuiltinFunction() const;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
