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

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

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153132 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 9055d143
2013-04-27 Filip Pizlo <fpizlo@apple.com>
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 <fpizlo@apple.com>
fourthTier: WatchpointSet should make racy uses easier to reason about
......
......@@ -3373,10 +3373,14 @@ bool CodeBlock::usesOpcode(OpcodeID opcodeID)
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");
......
......@@ -137,9 +137,10 @@ ParserError BytecodeGenerator::generate()
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 = &registerFor(result.iterator->value.getIndex());
......
......@@ -365,11 +365,12 @@ int ProgramExecutable::addGlobalVar(JSGlobalObject* globalObject, const Identifi
// 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();
......
......@@ -113,13 +113,16 @@ void JSActivation::getOwnNonIndexPropertyNames(JSObject* object, ExecState* exec
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 @@ inline bool JSActivation::symbolTablePutWithAttributes(VM& vm, PropertyName prop
{
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 = &registerAt(entry.getIndex());
}
reg->set(vm, this, value);
return true;
}
......
/*
* 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 @@ bool JSSymbolTableObject::deleteProperty(JSCell* cell, ExecState* exec, Property
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);
......
......@@ -73,8 +73,9 @@ inline bool symbolTableGet(
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 @@ inline bool symbolTableGet(
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 @@ inline bool symbolTableGet(
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 @@ inline bool symbolTablePut(
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 @@ inline bool symbolTablePutWithAttributes(
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;
}
......
......@@ -98,5 +98,8 @@ SymbolTableEntry::FatEntry* SymbolTableEntry::inflateSlow()
return entry;
}
SymbolTable::SymbolTable() { }
SymbolTable::~SymbolTable() { }
} // namespace JSC
......@@ -339,7 +339,103 @@ struct SymbolTableIndexHashTraits : HashTraits<SymbolTableEntry> {
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:
......
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