Commit 22fdb104 authored by oliver@apple.com's avatar oliver@apple.com

fourthTier: Create an equivalent of Structure::get() that can work from a compilation thread

https://bugs.webkit.org/show_bug.cgi?id=114987

Reviewed by Geoffrey Garen.

This completes the work started by r148570. That patch made it possible to do
Structure::get() without modifying Structure. This patch takes this further, and
makes this thread-safe (for non-uncacheable-dictionaries) via
Structure::getConcurrently(). This method not only doesn't modify Structure, but
also ensures that any concurrent attempts to add to, remove from, or steal the
table from that structure doesn't mess up the result of the call. The call may
return invalidOffset even if a property is *just* about to be added, but it will
never do the reverse: if it returns a property then you can be sure that the
structure really does have that property and always will have it.

* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFromLLInt):
(JSC::GetByIdStatus::computeForChain):
(JSC::GetByIdStatus::computeFor):
* bytecode/PutByIdStatus.cpp:
(JSC::PutByIdStatus::computeFromLLInt):
(JSC::PutByIdStatus::computeFor):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::isStringPrototypeMethodSane):
* runtime/PropertyMapHashTable.h:
(PropertyTable):
(JSC::PropertyTable::findConcurrently):
(JSC):
(JSC::PropertyTable::add):
(JSC::PropertyTable::remove):
(JSC::PropertyTable::reinsert):
(JSC::PropertyTable::rehash):
* runtime/PropertyTable.cpp:
(JSC::PropertyTable::PropertyTable):
* runtime/Structure.cpp:
(JSC::Structure::findStructuresAndMapForMaterialization):
(JSC::Structure::getConcurrently):
* runtime/Structure.h:
(Structure):
* runtime/StructureInlines.h:
(JSC::Structure::getConcurrently):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153128 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent a6b761cc
2013-07-16 Oliver Hunt <oliver@apple.com>
Merge dfgFourthTier r148936
2013-04-22 Filip Pizlo <fpizlo@apple.com>
fourthTier: Create an equivalent of Structure::get() that can work from a compilation thread
https://bugs.webkit.org/show_bug.cgi?id=114987
Reviewed by Geoffrey Garen.
This completes the work started by r148570. That patch made it possible to do
Structure::get() without modifying Structure. This patch takes this further, and
makes this thread-safe (for non-uncacheable-dictionaries) via
Structure::getConcurrently(). This method not only doesn't modify Structure, but
also ensures that any concurrent attempts to add to, remove from, or steal the
table from that structure doesn't mess up the result of the call. The call may
return invalidOffset even if a property is *just* about to be added, but it will
never do the reverse: if it returns a property then you can be sure that the
structure really does have that property and always will have it.
* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFromLLInt):
(JSC::GetByIdStatus::computeForChain):
(JSC::GetByIdStatus::computeFor):
* bytecode/PutByIdStatus.cpp:
(JSC::PutByIdStatus::computeFromLLInt):
(JSC::PutByIdStatus::computeFor):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::isStringPrototypeMethodSane):
* runtime/PropertyMapHashTable.h:
(PropertyTable):
(JSC::PropertyTable::findConcurrently):
(JSC):
(JSC::PropertyTable::add):
(JSC::PropertyTable::remove):
(JSC::PropertyTable::reinsert):
(JSC::PropertyTable::rehash):
* runtime/PropertyTable.cpp:
(JSC::PropertyTable::PropertyTable):
* runtime/Structure.cpp:
(JSC::Structure::findStructuresAndMapForMaterialization):
(JSC::Structure::getConcurrently):
* runtime/Structure.h:
(Structure):
* runtime/StructureInlines.h:
(JSC::Structure::getConcurrently):
2013-07-16 Oliver Hunt <oliver@apple.com>
Merge dfgFourthTier r148850
......
......@@ -51,7 +51,7 @@ GetByIdStatus GetByIdStatus::computeFromLLInt(CodeBlock* profiledBlock, unsigned
unsigned attributesIgnored;
JSCell* specificValue;
PropertyOffset offset = structure->get(
PropertyOffset offset = structure->getConcurrently(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (structure->isDictionary())
specificValue = 0;
......@@ -92,7 +92,7 @@ void GetByIdStatus::computeForChain(GetByIdStatus& result, CodeBlock* profiledBl
unsigned attributesIgnored;
JSCell* specificValue;
result.m_offset = currentStructure->get(
result.m_offset = currentStructure->getConcurrently(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (currentStructure->isDictionary())
specificValue = 0;
......@@ -166,7 +166,7 @@ GetByIdStatus GetByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
Structure* structure = stubInfo.u.getByIdSelf.baseObjectStructure.get();
unsigned attributesIgnored;
JSCell* specificValue;
result.m_offset = structure->getWithoutMaterializing(
result.m_offset = structure->getConcurrently(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (structure->isDictionary())
specificValue = 0;
......@@ -191,7 +191,7 @@ GetByIdStatus GetByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
unsigned attributesIgnored;
JSCell* specificValue;
PropertyOffset myOffset = structure->getWithoutMaterializing(
PropertyOffset myOffset = structure->getConcurrently(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (structure->isDictionary())
specificValue = 0;
......@@ -276,7 +276,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->getWithoutMaterializing(vm, ident, attributes, specificValue);
result.m_offset = structure->getConcurrently(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->getWithoutMaterializing(*profiledBlock->vm(), ident);
PropertyOffset offset = structure->getConcurrently(*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->getWithoutMaterializing(*profiledBlock->vm(), ident);
PropertyOffset offset = newStructure->getConcurrently(*profiledBlock->vm(), ident);
if (!isValidOffset(offset))
return PutByIdStatus(NoInformation, 0, 0, 0, invalidOffset);
......@@ -106,7 +106,7 @@ PutByIdStatus PutByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
case access_put_by_id_replace: {
PropertyOffset offset =
stubInfo.u.putByIdReplace.baseObjectStructure->getWithoutMaterializing(
stubInfo.u.putByIdReplace.baseObjectStructure->getConcurrently(
*profiledBlock->vm(), ident);
if (isValidOffset(offset)) {
return PutByIdStatus(
......@@ -122,7 +122,7 @@ PutByIdStatus PutByIdStatus::computeFor(CodeBlock* profiledBlock, unsigned bytec
case access_put_by_id_transition_direct: {
ASSERT(stubInfo.u.putByIdTransition.previousStructure->transitionWatchpointSetHasBeenInvalidated());
PropertyOffset offset =
stubInfo.u.putByIdTransition.structure->getWithoutMaterializing(
stubInfo.u.putByIdTransition.structure->getConcurrently(
*profiledBlock->vm(), ident);
if (isValidOffset(offset)) {
return PutByIdStatus(
......@@ -156,7 +156,7 @@ PutByIdStatus PutByIdStatus::computeFor(VM& vm, JSGlobalObject* globalObject, St
unsigned attributes;
JSCell* specificValue;
PropertyOffset offset = structure->getWithoutMaterializing(
PropertyOffset offset = structure->getConcurrently(
vm, ident, attributes, specificValue);
if (isValidOffset(offset)) {
if (attributes & (Accessor | ReadOnly))
......
......@@ -1083,7 +1083,7 @@ private:
{
unsigned attributesUnused;
JSCell* specificValue;
PropertyOffset offset = stringPrototypeStructure->get(
PropertyOffset offset = stringPrototypeStructure->getConcurrently(
vm(), ident, attributesUnused, specificValue);
if (!isValidOffset(offset))
return false;
......
......@@ -236,19 +236,27 @@ void Structure::destroy(JSCell* cell)
static_cast<Structure*>(cell)->Structure::~Structure();
}
void Structure::findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, PropertyTable*& table)
void Structure::findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, Structure*& structure, PropertyTable*& table)
{
ASSERT(structures.isEmpty());
table = 0;
for (Structure* structure = this; structure; structure = structure->previousID()) {
if (structure->propertyTable()) {
table = structure->propertyTable().get();
for (structure = this; structure; structure = structure->previousID()) {
structure->m_lock.lock();
table = structure->propertyTable().get();
if (table) {
// Leave the structure locked, so that the caller can do things to it atomically
// before it loses its property table.
return;
}
structures.append(structure);
structure->m_lock.unlock();
}
ASSERT(!structure);
ASSERT(!table);
}
void Structure::materializePropertyMap(VM& vm)
......@@ -257,17 +265,27 @@ void Structure::materializePropertyMap(VM& vm)
ASSERT(!propertyTable());
Vector<Structure*, 8> structures;
Structure* structure;
PropertyTable* table;
findStructuresAndMapForMaterialization(structures, table);
findStructuresAndMapForMaterialization(structures, structure, table);
if (table) {
table = table->copy(vm, 0, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity));
structure->m_lock.unlock();
}
// Must hold the lock on this structure, since we will be modifying this structure's
// property map. We don't want getConcurrently() to see the property map in a half-baked
// state.
Locker locker(m_lock);
if (!table)
createPropertyMap(vm, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity));
createPropertyMap(locker, vm, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity));
else
propertyTable().set(vm, this, table->copy(vm, 0, numberOfSlotsForLastOffset(m_offset, m_inlineCapacity)));
propertyTable().set(vm, this, table);
for (size_t i = structures.size(); i--;) {
Structure* structure = structures[i];
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());
......@@ -544,8 +562,14 @@ Structure* Structure::preventExtensionsTransition(VM& vm, Structure* structure)
PropertyTable* Structure::takePropertyTableOrCloneIfPinned(VM& vm, Structure* owner)
{
materializePropertyMapIfNecessaryForPinning(vm);
if (m_isPinnedPropertyTable)
return propertyTable()->copy(vm, owner, propertyTable()->size() + 1);
// Hold the lock while stealing the table - so that getConcurrently() on another thread
// will either have to bypass this structure, or will get to use the property table
// before it is stolen.
Locker locker(m_lock);
PropertyTable* takenPropertyTable = propertyTable().get();
propertyTable().clear();
return takenPropertyTable;
......@@ -747,24 +771,32 @@ 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)
PropertyOffset Structure::getConcurrently(VM&, PropertyName propertyName, unsigned& attributes, JSCell*& specificValue)
{
// We can't handle uncacheable dictionaries because we can't handle concurrent remove's
// from the property maps.
RELEASE_ASSERT(!isUncacheableDictionary());
Vector<Structure*, 8> structures;
Structure* structure;
PropertyTable* table;
findStructuresAndMapForMaterialization(structures, table);
findStructuresAndMapForMaterialization(structures, structure, table);
if (table) {
PropertyMapEntry* entry = table->find(propertyName.uid()).first;
if (entry) {
attributes = entry->attributes;
specificValue = entry->specificValue.get();
return entry->offset;
PropertyOffset result = entry->offset;
structure->m_lock.unlock();
return result;
}
structure->m_lock.unlock();
}
for (unsigned i = structures.size(); i--;) {
Structure* structure = structures[i];
structure = structures[i];
if (structure->m_nameInPrevious.get() != propertyName.uid())
continue;
......@@ -821,6 +853,8 @@ void Structure::despecifyAllFunctions(VM& vm)
PropertyOffset Structure::putSpecificValue(VM& vm, PropertyName propertyName, unsigned attributes, JSCell* specificValue)
{
Locker locker(m_lock);
ASSERT(!JSC::isValidOffset(get(vm, propertyName)));
checkConsistency();
......@@ -830,7 +864,7 @@ PropertyOffset Structure::putSpecificValue(VM& vm, PropertyName propertyName, un
StringImpl* rep = propertyName.uid();
if (!propertyTable())
createPropertyMap(vm);
createPropertyMap(locker, vm);
PropertyOffset newOffset = propertyTable()->nextOffset(m_inlineCapacity);
......@@ -842,6 +876,8 @@ PropertyOffset Structure::putSpecificValue(VM& vm, PropertyName propertyName, un
PropertyOffset Structure::remove(PropertyName propertyName)
{
Locker locker(m_lock);
checkConsistency();
StringImpl* rep = propertyName.uid();
......@@ -862,7 +898,7 @@ PropertyOffset Structure::remove(PropertyName propertyName)
return offset;
}
void Structure::createPropertyMap(VM& vm, unsigned capacity)
void Structure::createPropertyMap(const Locker&, VM& vm, unsigned capacity)
{
ASSERT(!propertyTable());
......
......@@ -39,6 +39,8 @@
#include "StructureTransitionTable.h"
#include "JSTypeInfo.h"
#include "Watchpoint.h"
#include "Weak.h"
#include <wtf/ByteSpinLock.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
#include <wtf/text/StringImpl.h>
......@@ -68,6 +70,9 @@ public:
friend class StructureTransitionTable;
typedef JSCell Base;
typedef ByteSpinLock Lock;
typedef ByteSpinLocker Locker;
static Structure* create(VM&, JSGlobalObject*, JSValue prototype, const TypeInfo&, const ClassInfo*, IndexingType = NonArray, unsigned inlineCapacity = 0);
......@@ -235,8 +240,8 @@ 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);
PropertyOffset getConcurrently(VM&, PropertyName);
PropertyOffset getConcurrently(VM&, PropertyName, unsigned& attributes, JSCell*& specificValue);
bool hasGetterSetterProperties() const { return m_hasGetterSetterProperties; }
bool hasReadOnlyOrGetterSetterPropertiesExcludingProto() const { return m_hasReadOnlyOrGetterSetterPropertiesExcludingProto; }
......@@ -360,8 +365,12 @@ private:
Structure(VM&, const Structure*);
static Structure* create(VM&, const Structure*);
void findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, PropertyTable*&);
// This will return the structure that has a usable property table, that property table,
// and the list of structures that we visited before we got to it. If it returns a
// non-null structure, it will also lock the structure that it returns; it is your job
// to unlock it.
void findStructuresAndMapForMaterialization(Vector<Structure*, 8>& structures, Structure*&, PropertyTable*&);
typedef enum {
NoneDictionaryKind = 0,
......@@ -373,7 +382,7 @@ private:
PropertyOffset putSpecificValue(VM&, PropertyName, unsigned attributes, JSCell* specificValue);
PropertyOffset remove(PropertyName);
void createPropertyMap(VM&, unsigned keyCount = 0);
void createPropertyMap(const Locker&, VM&, unsigned keyCount = 0);
void checkConsistency();
bool despecifyFunction(VM&, PropertyName);
......@@ -472,8 +481,10 @@ private:
TypeInfo m_typeInfo;
IndexingType m_indexingType;
uint8_t m_inlineCapacity;
Lock m_lock;
unsigned m_dictionaryKind : 2;
bool m_isPinnedPropertyTable : 1;
bool m_hasGetterSetterProperties : 1;
......
......@@ -78,11 +78,11 @@ inline PropertyOffset Structure::get(VM& vm, const WTF::String& name)
return entry ? entry->offset : invalidOffset;
}
inline PropertyOffset Structure::getWithoutMaterializing(VM& vm, PropertyName propertyName)
inline PropertyOffset Structure::getConcurrently(VM& vm, PropertyName propertyName)
{
unsigned attributesIgnored;
JSCell* specificValueIgnored;
return getWithoutMaterializing(
return getConcurrently(
vm, propertyName, attributesIgnored, specificValueIgnored);
}
......
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