Diff
Modified: branches/dfgFourthTier/Source/_javascript_Core/ChangeLog (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-04-27 23:14:41 UTC (rev 149248)
@@ -1,3 +1,43 @@
+2013-04-27 Filip Pizlo <[email protected]>
+
+ fourthTier: SymbolTable should be thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=115301
+
+ Reviewed by Geoffrey Garen.
+
+ Makes SymbolTable thread-safe. Relies on SymbolTableEntry already being immutable,
+ other than the WatchpointSet; but the WatchpointSet already has a righteous
+ concurrency protocol. So, this patch just protects the SymbolTable's HashMap.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::nameForRegister):
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::addVar):
+ * runtime/Executable.cpp:
+ (JSC::ProgramExecutable::addGlobalVar):
+ * runtime/JSActivation.cpp:
+ (JSC::JSActivation::getOwnNonIndexPropertyNames):
+ (JSC::JSActivation::symbolTablePutWithAttributes):
+ * runtime/JSSymbolTableObject.cpp:
+ (JSC::JSSymbolTableObject::getOwnNonIndexPropertyNames):
+ * runtime/JSSymbolTableObject.h:
+ (JSC::symbolTableGet):
+ (JSC::symbolTablePut):
+ (JSC::symbolTablePutWithAttributes):
+ * runtime/SymbolTable.cpp:
+ (JSC::SymbolTable::SymbolTable):
+ (JSC::SymbolTable::~SymbolTable):
+ * runtime/SymbolTable.h:
+ (JSC::SymbolTable::find):
+ (JSC::SymbolTable::get):
+ (JSC::SymbolTable::inlineGet):
+ (JSC::SymbolTable::begin):
+ (JSC::SymbolTable::end):
+ (JSC::SymbolTable::size):
+ (JSC::SymbolTable::add):
+ (JSC::SymbolTable::set):
+ (JSC::SymbolTable::contains):
+
2013-04-26 Filip Pizlo <[email protected]>
fourthTier: WatchpointSet should make racy uses easier to reason about
Modified: branches/dfgFourthTier/Source/_javascript_Core/bytecode/CodeBlock.cpp (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/bytecode/CodeBlock.cpp 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/bytecode/CodeBlock.cpp 2013-04-27 23:14:41 UTC (rev 149248)
@@ -3397,10 +3397,14 @@
String CodeBlock::nameForRegister(int registerNumber)
{
- SymbolTable::iterator end = symbolTable()->end();
- for (SymbolTable::iterator ptr = symbolTable()->begin(); ptr != end; ++ptr) {
- if (ptr->value.getIndex() == registerNumber)
+ SymbolTable::Locker locker(symbolTable()->m_lock);
+ SymbolTable::Map::iterator end = symbolTable()->end(locker);
+ for (SymbolTable::Map::iterator ptr = symbolTable()->begin(locker); ptr != end; ++ptr) {
+ if (ptr->value.getIndex() == registerNumber) {
+ // FIXME: This won't work from the compilation thread.
+ // https://bugs.webkit.org/show_bug.cgi?id=115300
return String(ptr->key);
+ }
}
if (needsActivation() && registerNumber == activationRegister())
return ASCIILiteral("activation");
Modified: branches/dfgFourthTier/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2013-04-27 23:14:41 UTC (rev 149248)
@@ -211,9 +211,10 @@
bool BytecodeGenerator::addVar(const Identifier& ident, bool isConstant, RegisterID*& r0)
{
+ SymbolTable::Locker locker(symbolTable().m_lock);
int index = m_calleeRegisters.size();
SymbolTableEntry newEntry(index, isConstant ? ReadOnly : 0);
- SymbolTable::AddResult result = symbolTable().add(ident.impl(), newEntry);
+ SymbolTable::Map::AddResult result = symbolTable().add(locker, ident.impl(), newEntry);
if (!result.isNewEntry) {
r0 = ®isterFor(result.iterator->value.getIndex());
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/Executable.cpp (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/Executable.cpp 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/Executable.cpp 2013-04-27 23:14:41 UTC (rev 149248)
@@ -363,11 +363,12 @@
// Try to share the symbolTable if possible
SharedSymbolTable* symbolTable = globalObject->symbolTable();
UNUSED_PARAM(functionMode);
- int index = symbolTable->size();
+ SymbolTable::Locker locker(symbolTable->m_lock);
+ int index = symbolTable->size(locker);
SymbolTableEntry newEntry(index, (constantMode == IsConstant) ? ReadOnly : 0);
if (functionMode == IsFunctionToSpecialize)
newEntry.attemptToWatch();
- SymbolTable::AddResult result = symbolTable->add(ident.impl(), newEntry);
+ SymbolTable::Map::AddResult result = symbolTable->add(locker, ident.impl(), newEntry);
if (!result.isNewEntry) {
result.iterator->value.notifyWrite();
index = result.iterator->value.getIndex();
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/JSActivation.cpp (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/JSActivation.cpp 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/JSActivation.cpp 2013-04-27 23:14:41 UTC (rev 149248)
@@ -113,13 +113,16 @@
if (mode == IncludeDontEnumProperties && !thisObject->isTornOff())
propertyNames.add(exec->propertyNames().arguments);
- SymbolTable::const_iterator end = thisObject->symbolTable()->end();
- for (SymbolTable::const_iterator it = thisObject->symbolTable()->begin(); it != end; ++it) {
- if (it->value.getAttributes() & DontEnum && mode != IncludeDontEnumProperties)
- continue;
- if (!thisObject->isValid(it->value))
- continue;
- propertyNames.add(Identifier(exec, it->key.get()));
+ {
+ SymbolTable::Locker locker(thisObject->symbolTable()->m_lock);
+ SymbolTable::Map::iterator end = thisObject->symbolTable()->end(locker);
+ for (SymbolTable::Map::iterator it = thisObject->symbolTable()->begin(locker); it != end; ++it) {
+ if (it->value.getAttributes() & DontEnum && mode != IncludeDontEnumProperties)
+ continue;
+ if (!thisObject->isValid(it->value))
+ continue;
+ propertyNames.add(Identifier(exec, it->key.get()));
+ }
}
// Skip the JSVariableObject implementation of getOwnNonIndexPropertyNames
JSObject::getOwnNonIndexPropertyNames(thisObject, exec, propertyNames, mode);
@@ -129,16 +132,21 @@
{
ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this));
- SymbolTable::iterator iter = symbolTable()->find(propertyName.publicName());
- if (iter == symbolTable()->end())
- return false;
- SymbolTableEntry& entry = iter->value;
- ASSERT(!entry.isNull());
- if (!isValid(entry))
- return false;
-
- entry.setAttributes(attributes);
- registerAt(entry.getIndex()).set(vm, this, value);
+ WriteBarrierBase<Unknown>* reg;
+ {
+ SymbolTable::Locker locker(symbolTable()->m_lock);
+ SymbolTable::Map::iterator iter = symbolTable()->find(locker, propertyName.publicName());
+ if (iter == symbolTable()->end(locker))
+ return false;
+ SymbolTableEntry& entry = iter->value;
+ ASSERT(!entry.isNull());
+ if (!isValid(entry))
+ return false;
+
+ entry.setAttributes(attributes);
+ reg = ®isterAt(entry.getIndex());
+ }
+ reg->set(vm, this, value);
return true;
}
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/JSSymbolTableObject.cpp (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/JSSymbolTableObject.cpp 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/JSSymbolTableObject.cpp 2013-04-27 23:14:41 UTC (rev 149248)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -60,10 +60,13 @@
void JSSymbolTableObject::getOwnNonIndexPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode)
{
JSSymbolTableObject* thisObject = jsCast<JSSymbolTableObject*>(object);
- SymbolTable::const_iterator end = thisObject->symbolTable()->end();
- for (SymbolTable::const_iterator it = thisObject->symbolTable()->begin(); it != end; ++it) {
- if (!(it->value.getAttributes() & DontEnum) || (mode == IncludeDontEnumProperties))
- propertyNames.add(Identifier(exec, it->key.get()));
+ {
+ SymbolTable::Locker locker(thisObject->symbolTable()->m_lock);
+ SymbolTable::Map::iterator end = thisObject->symbolTable()->end(locker);
+ for (SymbolTable::Map::iterator it = thisObject->symbolTable()->begin(locker); it != end; ++it) {
+ if (!(it->value.getAttributes() & DontEnum) || (mode == IncludeDontEnumProperties))
+ propertyNames.add(Identifier(exec, it->key.get()));
+ }
}
JSObject::getOwnNonIndexPropertyNames(thisObject, exec, propertyNames, mode);
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/JSSymbolTableObject.h (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/JSSymbolTableObject.h 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/JSSymbolTableObject.h 2013-04-27 23:14:41 UTC (rev 149248)
@@ -73,8 +73,9 @@
SymbolTableObjectType* object, PropertyName propertyName, PropertySlot& slot)
{
SymbolTable& symbolTable = *object->symbolTable();
- SymbolTable::iterator iter = symbolTable.find(propertyName.publicName());
- if (iter == symbolTable.end())
+ SymbolTable::Locker locker(symbolTable.m_lock);
+ SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.publicName());
+ if (iter == symbolTable.end(locker))
return false;
SymbolTableEntry::Fast entry = iter->value;
ASSERT(!entry.isNull());
@@ -87,8 +88,9 @@
SymbolTableObjectType* object, PropertyName propertyName, PropertyDescriptor& descriptor)
{
SymbolTable& symbolTable = *object->symbolTable();
- SymbolTable::iterator iter = symbolTable.find(propertyName.publicName());
- if (iter == symbolTable.end())
+ SymbolTable::Locker locker(symbolTable.m_lock);
+ SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.publicName());
+ if (iter == symbolTable.end(locker))
return false;
SymbolTableEntry::Fast entry = iter->value;
ASSERT(!entry.isNull());
@@ -103,8 +105,9 @@
bool& slotIsWriteable)
{
SymbolTable& symbolTable = *object->symbolTable();
- SymbolTable::iterator iter = symbolTable.find(propertyName.publicName());
- if (iter == symbolTable.end())
+ SymbolTable::Locker locker(symbolTable.m_lock);
+ SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.publicName());
+ if (iter == symbolTable.end(locker))
return false;
SymbolTableEntry::Fast entry = iter->value;
ASSERT(!entry.isNull());
@@ -121,21 +124,29 @@
VM& vm = exec->vm();
ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(object));
- SymbolTable& symbolTable = *object->symbolTable();
- SymbolTable::iterator iter = symbolTable.find(propertyName.publicName());
- if (iter == symbolTable.end())
- return false;
- bool wasFat;
- SymbolTableEntry::Fast fastEntry = iter->value.getFast(wasFat);
- ASSERT(!fastEntry.isNull());
- if (fastEntry.isReadOnly()) {
- if (shouldThrow)
- throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
- return true;
+ WriteBarrierBase<Unknown>* reg;
+ {
+ SymbolTable& symbolTable = *object->symbolTable();
+ SymbolTable::Locker locker(symbolTable.m_lock);
+ SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.publicName());
+ if (iter == symbolTable.end(locker))
+ return false;
+ bool wasFat;
+ SymbolTableEntry::Fast fastEntry = iter->value.getFast(wasFat);
+ ASSERT(!fastEntry.isNull());
+ if (fastEntry.isReadOnly()) {
+ if (shouldThrow)
+ throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
+ return true;
+ }
+ if (UNLIKELY(wasFat))
+ iter->value.notifyWrite();
+ reg = &object->registerAt(fastEntry.getIndex());
}
- if (UNLIKELY(wasFat))
- iter->value.notifyWrite();
- object->registerAt(fastEntry.getIndex()).set(vm, object, value);
+ // I'd prefer we not hold lock while executing barriers, since I prefer to reserve
+ // the right for barriers to be able to trigger GC. And I don't want to hold VM
+ // locks while GC'ing.
+ reg->set(vm, object, value);
return true;
}
@@ -145,15 +156,21 @@
JSValue value, unsigned attributes)
{
ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(object));
-
- SymbolTable::iterator iter = object->symbolTable()->find(propertyName.publicName());
- if (iter == object->symbolTable()->end())
- return false;
- SymbolTableEntry& entry = iter->value;
- ASSERT(!entry.isNull());
- entry.notifyWrite();
- entry.setAttributes(attributes);
- object->registerAt(entry.getIndex()).set(vm, object, value);
+
+ WriteBarrierBase<Unknown>* reg;
+ {
+ SymbolTable& symbolTable = *object->symbolTable();
+ SymbolTable::Locker locker(symbolTable.m_lock);
+ SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.publicName());
+ if (iter == symbolTable.end(locker))
+ return false;
+ SymbolTableEntry& entry = iter->value;
+ ASSERT(!entry.isNull());
+ entry.notifyWrite();
+ entry.setAttributes(attributes);
+ reg = &object->registerAt(entry.getIndex());
+ }
+ reg->set(vm, object, value);
return true;
}
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/SymbolTable.cpp (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/SymbolTable.cpp 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/SymbolTable.cpp 2013-04-27 23:14:41 UTC (rev 149248)
@@ -98,5 +98,8 @@
return entry;
}
+SymbolTable::SymbolTable() { }
+SymbolTable::~SymbolTable() { }
+
} // namespace JSC
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/SymbolTable.h (149247 => 149248)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/SymbolTable.h 2013-04-27 23:14:04 UTC (rev 149247)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/SymbolTable.h 2013-04-27 23:14:41 UTC (rev 149248)
@@ -340,8 +340,104 @@
static const bool needsDestruction = true;
};
-typedef HashMap<RefPtr<StringImpl>, SymbolTableEntry, IdentifierRepHash, HashTraits<RefPtr<StringImpl> >, SymbolTableIndexHashTraits> SymbolTable;
+class SymbolTable {
+public:
+ typedef HashMap<RefPtr<StringImpl>, SymbolTableEntry, IdentifierRepHash, HashTraits<RefPtr<StringImpl> >, SymbolTableIndexHashTraits> Map;
+ typedef ByteSpinLock Lock;
+ typedef ByteSpinLocker Locker;
+ JS_EXPORT_PRIVATE SymbolTable();
+ JS_EXPORT_PRIVATE ~SymbolTable();
+
+ // You must hold the lock until after you're done with the iterator.
+ Map::iterator find(const Locker&, StringImpl* key)
+ {
+ return m_map.find(key);
+ }
+
+ SymbolTableEntry get(const Locker&, StringImpl* key)
+ {
+ return m_map.get(key);
+ }
+
+ SymbolTableEntry get(StringImpl* key)
+ {
+ Locker locker(m_lock);
+ return get(locker, key);
+ }
+
+ SymbolTableEntry inlineGet(const Locker&, StringImpl* key)
+ {
+ return m_map.inlineGet(key);
+ }
+
+ SymbolTableEntry inlineGet(StringImpl* key)
+ {
+ Locker locker(m_lock);
+ return inlineGet(locker, key);
+ }
+
+ Map::iterator begin(const Locker&)
+ {
+ return m_map.begin();
+ }
+
+ Map::iterator end(const Locker&)
+ {
+ return m_map.end();
+ }
+
+ size_t size(const Locker&) const
+ {
+ return m_map.size();
+ }
+
+ size_t size() const
+ {
+ Locker locker(m_lock);
+ return size(locker);
+ }
+
+ Map::AddResult add(const Locker&, StringImpl* key, const SymbolTableEntry& entry)
+ {
+ return m_map.add(key, entry);
+ }
+
+ void add(StringImpl* key, const SymbolTableEntry& entry)
+ {
+ Locker locker(m_lock);
+ add(locker, key, entry);
+ }
+
+ Map::AddResult set(const Locker&, StringImpl* key, const SymbolTableEntry& entry)
+ {
+ return m_map.set(key, entry);
+ }
+
+ void set(StringImpl* key, const SymbolTableEntry& entry)
+ {
+ Locker locker(m_lock);
+ set(locker, key, entry);
+ }
+
+ bool contains(const Locker&, StringImpl* key)
+ {
+ return m_map.contains(key);
+ }
+
+ bool contains(StringImpl* key)
+ {
+ Locker locker(m_lock);
+ return contains(locker, key);
+ }
+
+private:
+ Map m_map;
+public:
+ mutable ByteSpinLock m_lock;
+};
+
+
class SharedSymbolTable : public JSCell, public SymbolTable {
public:
typedef JSCell Base;
