Diff
Modified: branches/dfgFourthTier/Source/_javascript_Core/ChangeLog (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-04-23 00:16:27 UTC (rev 148936)
@@ -1,3 +1,47 @@
+2013-04-22 Filip Pizlo <fpi...@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-04-21 Filip Pizlo <fpi...@apple.com>
fourthTier: WebKit's build system should relink _javascript_Core if LLVM's libraries changed but its headers didn't
Modified: branches/dfgFourthTier/Source/_javascript_Core/bytecode/GetByIdStatus.cpp (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2013-04-23 00:16:27 UTC (rev 148936)
@@ -51,7 +51,7 @@
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 @@
unsigned attributesIgnored;
JSCell* specificValue;
- result.m_offset = currentStructure->get(
+ result.m_offset = currentStructure->getConcurrently(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (currentStructure->isDictionary())
specificValue = 0;
@@ -163,7 +163,7 @@
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;
@@ -188,7 +188,7 @@
unsigned attributesIgnored;
JSCell* specificValue;
- PropertyOffset myOffset = structure->getWithoutMaterializing(
+ PropertyOffset myOffset = structure->getConcurrently(
*profiledBlock->vm(), ident, attributesIgnored, specificValue);
if (structure->isDictionary())
specificValue = 0;
@@ -273,7 +273,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->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)
Modified: branches/dfgFourthTier/Source/_javascript_Core/bytecode/PutByIdStatus.cpp (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2013-04-23 00:16:27 UTC (rev 148936)
@@ -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->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 @@
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);
@@ -102,7 +102,7 @@
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(
@@ -118,7 +118,7 @@
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(
@@ -152,7 +152,7 @@
unsigned attributes;
JSCell* specificValueIgnored;
- PropertyOffset offset = structure->getWithoutMaterializing(
+ PropertyOffset offset = structure->getConcurrently(
vm, ident, attributes, specificValueIgnored);
if (isValidOffset(offset)) {
if (attributes & (Accessor | ReadOnly))
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-04-23 00:16:27 UTC (rev 148936)
@@ -1021,7 +1021,7 @@
{
unsigned attributesUnused;
JSCell* specificValue;
- PropertyOffset offset = stringPrototypeStructure->get(
+ PropertyOffset offset = stringPrototypeStructure->getConcurrently(
vm(), ident, attributesUnused, specificValue);
if (!isValidOffset(offset))
return false;
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/Structure.cpp (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/Structure.cpp 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/Structure.cpp 2013-04-23 00:16:27 UTC (rev 148936)
@@ -236,19 +236,27 @@
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 @@
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 @@
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 @@
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 @@
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 @@
StringImpl* rep = propertyName.uid();
if (!propertyTable())
- createPropertyMap(vm);
+ createPropertyMap(locker, vm);
PropertyOffset newOffset = propertyTable()->nextOffset(m_inlineCapacity);
@@ -842,6 +876,8 @@
PropertyOffset Structure::remove(PropertyName propertyName)
{
+ Locker locker(m_lock);
+
checkConsistency();
StringImpl* rep = propertyName.uid();
@@ -862,7 +898,7 @@
return offset;
}
-void Structure::createPropertyMap(VM& vm, unsigned capacity)
+void Structure::createPropertyMap(const Locker&, VM& vm, unsigned capacity)
{
ASSERT(!propertyTable());
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/Structure.h (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/Structure.h 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/Structure.h 2013-04-23 00:16:27 UTC (rev 148936)
@@ -40,6 +40,7 @@
#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>
@@ -69,6 +70,9 @@
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);
@@ -236,8 +240,8 @@
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; }
@@ -361,9 +365,13 @@
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,
CachedDictionaryKind = 1,
@@ -374,7 +382,7 @@
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);
@@ -473,8 +481,10 @@
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;
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/StructureInlines.h (148935 => 148936)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/StructureInlines.h 2013-04-23 00:16:26 UTC (rev 148935)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/StructureInlines.h 2013-04-23 00:16:27 UTC (rev 148936)
@@ -78,11 +78,11 @@
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);
}
