Commit b3956443 authored by oliver@apple.com's avatar oliver@apple.com

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):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153120 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent b9009149
2013-07-16 Oliver Hunt <oliver@apple.com>
Merged dfgFourthTier r148570
2013-04-16 Filip Pizlo <fpizlo@apple.com>
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 <oliver@apple.com>
Merged dfgFourthTier r148047
......
......@@ -164,7 +164,7 @@ GetByIdStatus GetByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
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 @@ GetByIdStatus GetByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
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 @@ GetByIdStatus GetByIdStatus::computeFor(VM& vm, Structure* structure, Identifier
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)
......
......@@ -49,7 +49,7 @@ PutByIdStatus PutByIdStatus::computeFromLLInt(CodeBlock* profiledBlock, unsigned
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 @@ PutByIdStatus PutByIdStatus::computeFromLLInt(CodeBlock* profiledBlock, unsigned
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 @@ PutByIdStatus PutByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
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 @@ PutByIdStatus PutByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
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 @@ PutByIdStatus PutByIdStatus::computeFor(VM& vm, JSGlobalObject* globalObject, St
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);
......
......@@ -236,34 +236,38 @@ void Structure::destroy(JSCell* cell)
static_cast<Structure*>(cell)->Structure::~Structure();
}
void Structure::materializePropertyMap(VM& vm)
void Structure::findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, PropertyTable*& table)
{
ASSERT(structure()->classInfo() == &s_info);
ASSERT(!propertyTable());
Vector<Structure*, 8> structures;
structures.append(this);
ASSERT(structures.isEmpty());
table = 0;
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;
for (Structure* structure = this; structure; structure = structure->previousID()) {
if (structure->propertyTable()) {
table = structure->propertyTable().get();
return;
}
structures.append(structure);
}
}
if (!propertyTable())
void Structure::materializePropertyMap(VM& vm)
{
ASSERT(structure()->classInfo() == &s_info);
ASSERT(!propertyTable());
Vector<Structure*, 8> structures;
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 @@ PropertyTable* Structure::copyPropertyTableForPinning(VM& vm, Structure* owner)
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);
......
......@@ -235,6 +235,9 @@ public:
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 @@ private:
static Structure* create(VM&, const Structure*);
void findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, PropertyTable*&);
typedef enum {
NoneDictionaryKind = 0,
CachedDictionaryKind = 1,
......
......@@ -78,6 +78,14 @@ inline PropertyOffset Structure::get(VM& vm, const WTF::String& name)
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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment