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

fourthTier: Structure::addPropertyTransitionToExistingStructure should be thread-safe

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

Reviewed by Geoffrey Garen.

This makes the main thread modify the transition table while holding a lock. Note
that the GC might modify its weak pointers without locking, but the GC will lock out
the compilation thread anyway. The map will then only reshape in response to add()
and set(), which happen while holding a lock.

This allows the compilation thread to now query transition tables safely, provided it
holds a lock when doing so.

Also changed LLVM asm printer initialization to just initialize the X86 one. It makes
sense for us to just initialize the asm printer(s) that we actually use; you could
imagine us being linked to a system LLVM that has cross-compilation support; there is
no point in the WebKit or JSC process doing work to initialize all of those targets.
That part was rubber stamped by Mark Hahnenberg.

* bytecode/PutByIdStatus.cpp:
(JSC::PutByIdStatus::computeFor):
* runtime/InitializeThreading.cpp:
(JSC::initializeThreadingOnce):
* runtime/Structure.cpp:
(JSC::Structure::addPropertyTransitionToExistingStructureImpl):
(JSC::Structure::addPropertyTransitionToExistingStructure):
(JSC):
(JSC::Structure::addPropertyTransitionToExistingStructureConcurrently):
(JSC::Structure::addPropertyTransition):
(JSC::Structure::nonPropertyTransition):
* runtime/Structure.h:
(Structure):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153140 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ac8aba9a
2013-05-01 Filip Pizlo <fpizlo@apple.com>
fourthTier: Structure::addPropertyTransitionToExistingStructure should be thread-safe
https://bugs.webkit.org/show_bug.cgi?id=115468
Reviewed by Geoffrey Garen.
This makes the main thread modify the transition table while holding a lock. Note
that the GC might modify its weak pointers without locking, but the GC will lock out
the compilation thread anyway. The map will then only reshape in response to add()
and set(), which happen while holding a lock.
This allows the compilation thread to now query transition tables safely, provided it
holds a lock when doing so.
Also changed LLVM asm printer initialization to just initialize the X86 one. It makes
sense for us to just initialize the asm printer(s) that we actually use; you could
imagine us being linked to a system LLVM that has cross-compilation support; there is
no point in the WebKit or JSC process doing work to initialize all of those targets.
That part was rubber stamped by Mark Hahnenberg.
* bytecode/PutByIdStatus.cpp:
(JSC::PutByIdStatus::computeFor):
* runtime/InitializeThreading.cpp:
(JSC::initializeThreadingOnce):
* runtime/Structure.cpp:
(JSC::Structure::addPropertyTransitionToExistingStructureImpl):
(JSC::Structure::addPropertyTransitionToExistingStructure):
(JSC):
(JSC::Structure::addPropertyTransitionToExistingStructureConcurrently):
(JSC::Structure::addPropertyTransition):
(JSC::Structure::nonPropertyTransition):
* runtime/Structure.h:
(Structure):
2013-04-30 Filip Pizlo <fpizlo@apple.com>
fourthTier: Structure::getConcurrently() may be called from for uncacheable dictionaries, and this is safe
......
......@@ -210,7 +210,7 @@ PutByIdStatus PutByIdStatus::computeFor(VM& vm, JSGlobalObject* globalObject, St
// - If we're not storing a value that could be specific: again, this would only be a
// problem if the existing transition did have a specific value, which we check for
// by passing 0 for the specificValue.
Structure* transition = Structure::addPropertyTransitionToExistingStructure(structure, ident, 0, 0, offset);
Structure* transition = Structure::addPropertyTransitionToExistingStructureConcurrently(structure, ident, 0, 0, offset);
if (!transition)
return PutByIdStatus(TakesSlowPath); // This occurs in bizarre cases only. See above.
ASSERT(!transition->transitionDidInvolveSpecificValue());
......
......@@ -73,7 +73,7 @@ static void initializeThreadingOnce()
#if ENABLE(FTL_JIT)
LLVMLinkInMCJIT();
LLVMInitializeNativeTarget();
LLVMInitializeAllAsmPrinters();
LLVMInitializeX86AsmPrinter();
#endif
}
......
......@@ -321,7 +321,7 @@ void Structure::despecifyDictionaryFunction(VM& vm, PropertyName propertyName)
entry->specificValue.clear();
}
Structure* Structure::addPropertyTransitionToExistingStructure(Structure* structure, PropertyName propertyName, unsigned attributes, JSCell* specificValue, PropertyOffset& offset)
Structure* Structure::addPropertyTransitionToExistingStructureImpl(Structure* structure, PropertyName propertyName, unsigned attributes, JSCell* specificValue, PropertyOffset& offset)
{
ASSERT(!structure->isDictionary());
ASSERT(structure->isObject());
......@@ -338,6 +338,18 @@ Structure* Structure::addPropertyTransitionToExistingStructure(Structure* struct
return 0;
}
Structure* Structure::addPropertyTransitionToExistingStructure(Structure* structure, PropertyName propertyName, unsigned attributes, JSCell* specificValue, PropertyOffset& offset)
{
ASSERT(!isCompilationThread());
return addPropertyTransitionToExistingStructureImpl(structure, propertyName, attributes, specificValue, offset);
}
Structure* Structure::addPropertyTransitionToExistingStructureConcurrently(Structure* structure, PropertyName propertyName, unsigned attributes, JSCell* specificValue, PropertyOffset& offset)
{
Locker locker(structure->m_lock);
return addPropertyTransitionToExistingStructureImpl(structure, propertyName, attributes, specificValue, offset);
}
bool Structure::anyObjectInChainMayInterceptIndexedAccesses() const
{
for (const Structure* current = this; ;) {
......@@ -405,7 +417,10 @@ Structure* Structure::addPropertyTransition(VM& vm, Structure* structure, Proper
offset = transition->putSpecificValue(vm, propertyName, attributes, specificValue);
checkOffset(transition->m_offset, transition->inlineCapacity());
structure->m_transitionTable.add(vm, transition);
{
Locker locker(structure->m_lock);
structure->m_transitionTable.add(vm, transition);
}
transition->checkOffsetConsistency();
structure->checkOffsetConsistency();
return transition;
......@@ -604,7 +619,10 @@ Structure* Structure::nonPropertyTransition(VM& vm, Structure* structure, NonPro
transition->m_offset = structure->m_offset;
checkOffset(transition->m_offset, transition->inlineCapacity());
structure->m_transitionTable.add(vm, transition);
{
Locker locker(structure->m_lock);
structure->m_transitionTable.add(vm, transition);
}
transition->checkOffsetConsistency();
return transition;
}
......
......@@ -97,6 +97,7 @@ public:
static void dumpStatistics();
JS_EXPORT_PRIVATE static Structure* addPropertyTransition(VM&, Structure*, PropertyName, unsigned attributes, JSCell* specificValue, PropertyOffset&);
static Structure* addPropertyTransitionToExistingStructureConcurrently(Structure*, PropertyName, unsigned attributes, JSCell* specificValue, PropertyOffset&);
JS_EXPORT_PRIVATE static Structure* addPropertyTransitionToExistingStructure(Structure*, PropertyName, unsigned attributes, JSCell* specificValue, PropertyOffset&);
static Structure* removePropertyTransition(VM&, Structure*, PropertyName, PropertyOffset&);
JS_EXPORT_PRIVATE static Structure* changePrototypeTransition(VM&, Structure*, JSValue prototype);
......@@ -367,6 +368,8 @@ private:
static Structure* create(VM&, const Structure*);
static Structure* addPropertyTransitionToExistingStructureImpl(Structure*, PropertyName, unsigned attributes, JSCell* specificValue, PropertyOffset&);
// 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
......
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