- Revision
- 153120
- Author
- [email protected]
- Date
- 2013-07-24 20:58:27 -0700 (Wed, 24 Jul 2013)
Log Message
fourthTier: DFG should be able to query Structure without modifying it
https://bugs.webkit.org/show_bug.cgi?id=114708
Reviewed by Oliver Hunt.
This is work towards allowing the DFG, and FTL, to run on a separate thread.
The idea is that the most evil thing that the DFG does that has thread-safety
issues is fiddling with Structures by calling Structure::get(). This can lead
to rematerialization of property tables, which is definitely not thread-safe
due to how StringImpl works. So, this patch completely side-steps the problem
by creating a new version of Structure::get, called
Structure::getWithoutMaterializing, which may choose to do an O(n) search if
necessary to avoid materialization. I believe this should be fine - the DFG
does't call into these code path often enough for this to matter, and most of
the time, the Structure that we call this on will already have a property
table because some inline cache would have already called ::get() on that
Structure.
Also cleaned up the materialization logic: we can stop the search as soon as
we find any Structure with a property table rather than searching all the way
for a pinned one.
* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFor):
* bytecode/PutByIdStatus.cpp:
(JSC::PutByIdStatus::computeFromLLInt):
(JSC::PutByIdStatus::computeFor):
* runtime/Structure.cpp:
(JSC::Structure::findStructuresAndMapForMaterialization):
(JSC::Structure::materializePropertyMap):
(JSC::Structure::getWithoutMaterializing):
(JSC):
* runtime/Structure.h:
(Structure):
* runtime/StructureInlines.h:
(JSC::Structure::getWithoutMaterializing):
(JSC):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (153119 => 153120)
--- trunk/Source/_javascript_Core/ChangeLog 2013-07-25 03:58:26 UTC (rev 153119)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-07-25 03:58:27 UTC (rev 153120)
@@ -1,3 +1,47 @@
+2013-07-16 Oliver Hunt <[email protected]>
+
+ Merged dfgFourthTier r148570
+
+ 2013-04-16 Filip Pizlo <[email protected]>
+
+ fourthTier: DFG should be able to query Structure without modifying it
+ https://bugs.webkit.org/show_bug.cgi?id=114708
+
+ Reviewed by Oliver Hunt.
+
+ This is work towards allowing the DFG, and FTL, to run on a separate thread.
+ The idea is that the most evil thing that the DFG does that has thread-safety
+ issues is fiddling with Structures by calling Structure::get(). This can lead
+ to rematerialization of property tables, which is definitely not thread-safe
+ due to how StringImpl works. So, this patch completely side-steps the problem
+ by creating a new version of Structure::get, called
+ Structure::getWithoutMaterializing, which may choose to do an O(n) search if
+ necessary to avoid materialization. I believe this should be fine - the DFG
+ does't call into these code path often enough for this to matter, and most of
+ the time, the Structure that we call this on will already have a property
+ table because some inline cache would have already called ::get() on that
+ Structure.
+
+ Also cleaned up the materialization logic: we can stop the search as soon as
+ we find any Structure with a property table rather than searching all the way
+ for a pinned one.
+
+ * bytecode/GetByIdStatus.cpp:
+ (JSC::GetByIdStatus::computeFor):
+ * bytecode/PutByIdStatus.cpp:
+ (JSC::PutByIdStatus::computeFromLLInt):
+ (JSC::PutByIdStatus::computeFor):
+ * runtime/Structure.cpp:
+ (JSC::Structure::findStructuresAndMapForMaterialization):
+ (JSC::Structure::materializePropertyMap):
+ (JSC::Structure::getWithoutMaterializing):
+ (JSC):
+ * runtime/Structure.h:
+ (Structure):
+ * runtime/StructureInlines.h:
+ (JSC::Structure::getWithoutMaterializing):
+ (JSC):
+
2013-07-15 Oliver Hunt <[email protected]>
Merged dfgFourthTier r148047
Modified: trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp (153119 => 153120)
--- trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2013-07-25 03:58:26 UTC (rev 153119)
+++ trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2013-07-25 03:58:27 UTC (rev 153120)
@@ -164,7 +164,7 @@
Structure* structure = stubInfo.u.getByIdSelf.baseObjectStructure.get();
unsigned attributesIgnored;
JSCell* specificValue;
- result.m_offset = structure->get(
+ result.m_offset = structure->getWithoutMaterializing(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (structure->isDictionary())
specificValue = 0;
@@ -189,7 +189,7 @@
unsigned attributesIgnored;
JSCell* specificValue;
- PropertyOffset myOffset = structure->get(
+ PropertyOffset myOffset = structure->getWithoutMaterializing(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (structure->isDictionary())
specificValue = 0;
@@ -274,7 +274,7 @@
result.m_wasSeenInJIT = false; // To my knowledge nobody that uses computeFor(VM&, Structure*, Identifier&) reads this field, but I might as well be honest: no, it wasn't seen in the JIT, since I computed it statically.
unsigned attributes;
JSCell* specificValue;
- result.m_offset = structure->get(vm, ident, attributes, specificValue);
+ result.m_offset = structure->getWithoutMaterializing(vm, ident, attributes, specificValue);
if (!isValidOffset(result.m_offset))
return GetByIdStatus(TakesSlowPath); // It's probably a prototype lookup. Give up on life for now, even though we could totally be way smarter about it.
if (attributes & Accessor)
Modified: trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp (153119 => 153120)
--- trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2013-07-25 03:58:26 UTC (rev 153119)
+++ trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2013-07-25 03:58:27 UTC (rev 153120)
@@ -49,7 +49,7 @@
if (instruction[0].u.opcode == LLInt::getOpcode(llint_op_put_by_id)
|| instruction[0].u.opcode == LLInt::getOpcode(llint_op_put_by_id_out_of_line)) {
- PropertyOffset offset = structure->get(*profiledBlock->vm(), ident);
+ PropertyOffset offset = structure->getWithoutMaterializing(*profiledBlock->vm(), ident);
if (!isValidOffset(offset))
return PutByIdStatus(NoInformation, 0, 0, 0, invalidOffset);
@@ -68,7 +68,7 @@
ASSERT(newStructure);
ASSERT(chain);
- PropertyOffset offset = newStructure->get(*profiledBlock->vm(), ident);
+ PropertyOffset offset = newStructure->getWithoutMaterializing(*profiledBlock->vm(), ident);
if (!isValidOffset(offset))
return PutByIdStatus(NoInformation, 0, 0, 0, invalidOffset);
@@ -103,8 +103,9 @@
return PutByIdStatus(TakesSlowPath, 0, 0, 0, invalidOffset);
case access_put_by_id_replace: {
- PropertyOffset offset = stubInfo.u.putByIdReplace.baseObjectStructure->get(
- *profiledBlock->vm(), ident);
+ PropertyOffset offset =
+ stubInfo.u.putByIdReplace.baseObjectStructure->getWithoutMaterializing(
+ *profiledBlock->vm(), ident);
if (isValidOffset(offset)) {
return PutByIdStatus(
SimpleReplace,
@@ -118,8 +119,9 @@
case access_put_by_id_transition_normal:
case access_put_by_id_transition_direct: {
ASSERT(stubInfo.u.putByIdTransition.previousStructure->transitionWatchpointSetHasBeenInvalidated());
- PropertyOffset offset = stubInfo.u.putByIdTransition.structure->get(
- *profiledBlock->vm(), ident);
+ PropertyOffset offset =
+ stubInfo.u.putByIdTransition.structure->getWithoutMaterializing(
+ *profiledBlock->vm(), ident);
if (isValidOffset(offset)) {
return PutByIdStatus(
SimpleTransition,
@@ -152,7 +154,8 @@
unsigned attributes;
JSCell* specificValue;
- PropertyOffset offset = structure->get(vm, ident, attributes, specificValue);
+ PropertyOffset offset = structure->getWithoutMaterializing(
+ vm, ident, attributes, specificValue);
if (isValidOffset(offset)) {
if (attributes & (Accessor | ReadOnly))
return PutByIdStatus(TakesSlowPath);
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (153119 => 153120)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2013-07-25 03:58:26 UTC (rev 153119)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2013-07-25 03:58:27 UTC (rev 153120)
@@ -236,34 +236,38 @@
static_cast<Structure*>(cell)->Structure::~Structure();
}
+void Structure::findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, PropertyTable*& table)
+{
+ ASSERT(structures.isEmpty());
+ table = 0;
+
+ for (Structure* structure = this; structure; structure = structure->previousID()) {
+ if (structure->propertyTable()) {
+ table = structure->propertyTable().get();
+ return;
+ }
+
+ structures.append(structure);
+ }
+}
+
void Structure::materializePropertyMap(VM& vm)
{
ASSERT(structure()->classInfo() == &s_info);
ASSERT(!propertyTable());
Vector<Structure*, 8> structures;
- structures.append(this);
-
- Structure* structure = this;
-
- // Search for the last Structure with a property table.
- while ((structure = structure->previousID())) {
- if (structure->m_isPinnedPropertyTable) {
- ASSERT(structure->propertyTable());
- ASSERT(!structure->previousID());
-
- propertyTable().set(vm, this, structure->propertyTable()->copy(vm, 0, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity)));
- break;
- }
-
- structures.append(structure);
- }
-
- if (!propertyTable())
+ PropertyTable* table;
+
+ findStructuresAndMapForMaterialization(structures, table);
+
+ if (!table)
createPropertyMap(vm, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity));
+ else
+ propertyTable().set(vm, this, table->copy(vm, 0, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity)));
- for (ptrdiff_t i = structures.size() - 1; i >= 0; --i) {
- structure = structures[i];
+ for (size_t i = structures.size(); i--;) {
+ Structure* structure = structures[i];
if (!structure->m_nameInPrevious)
continue;
PropertyMapEntry entry(vm, this, structure->m_nameInPrevious.get(), structure->m_offset, structure->m_attributesInPrevious, structure->m_specificValueInPrevious.get());
@@ -743,6 +747,35 @@
return PropertyTable::create(vm, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity));
}
+PropertyOffset Structure::getWithoutMaterializing(VM&, PropertyName propertyName, unsigned& attributes, JSCell*& specificValue)
+{
+ Vector<Structure*, 8> structures;
+ PropertyTable* table;
+
+ findStructuresAndMapForMaterialization(structures, table);
+
+ if (table) {
+ PropertyMapEntry* entry = table->find(propertyName.uid()).first;
+ if (entry) {
+ attributes = entry->attributes;
+ specificValue = entry->specificValue.get();
+ return entry->offset;
+ }
+ }
+
+ for (unsigned i = structures.size(); i--;) {
+ Structure* structure = structures[i];
+ if (structure->m_nameInPrevious.get() != propertyName.uid())
+ continue;
+
+ attributes = structure->m_attributesInPrevious;
+ specificValue = structure->m_specificValueInPrevious.get();
+ return structure->m_offset;
+ }
+
+ return invalidOffset;
+}
+
PropertyOffset Structure::get(VM& vm, PropertyName propertyName, unsigned& attributes, JSCell*& specificValue)
{
ASSERT(structure()->classInfo() == &s_info);
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (153119 => 153120)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2013-07-25 03:58:26 UTC (rev 153119)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2013-07-25 03:58:27 UTC (rev 153120)
@@ -235,6 +235,9 @@
PropertyOffset get(VM&, const WTF::String& name);
JS_EXPORT_PRIVATE PropertyOffset get(VM&, PropertyName, unsigned& attributes, JSCell*& specificValue);
+ PropertyOffset getWithoutMaterializing(VM&, PropertyName);
+ PropertyOffset getWithoutMaterializing(VM&, PropertyName, unsigned& attributes, JSCell*& specificValue);
+
bool hasGetterSetterProperties() const { return m_hasGetterSetterProperties; }
bool hasReadOnlyOrGetterSetterPropertiesExcludingProto() const { return m_hasReadOnlyOrGetterSetterPropertiesExcludingProto; }
void setHasGetterSetterProperties(bool is__proto__)
@@ -353,6 +356,8 @@
static Structure* create(VM&, const Structure*);
+ void findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, PropertyTable*&);
+
typedef enum {
NoneDictionaryKind = 0,
CachedDictionaryKind = 1,
Modified: trunk/Source/_javascript_Core/runtime/StructureInlines.h (153119 => 153120)
--- trunk/Source/_javascript_Core/runtime/StructureInlines.h 2013-07-25 03:58:26 UTC (rev 153119)
+++ trunk/Source/_javascript_Core/runtime/StructureInlines.h 2013-07-25 03:58:27 UTC (rev 153120)
@@ -78,6 +78,14 @@
return entry ? entry->offset : invalidOffset;
}
+inline PropertyOffset Structure::getWithoutMaterializing(VM& vm, PropertyName propertyName)
+{
+ unsigned attributesIgnored;
+ JSCell* specificValueIgnored;
+ return getWithoutMaterializing(
+ vm, propertyName, attributesIgnored, specificValueIgnored);
+}
+
inline bool Structure::masqueradesAsUndefined(JSGlobalObject* lexicalGlobalObject)
{
return typeInfo().masqueradesAsUndefined() && globalObject() == lexicalGlobalObject;
