- Revision
- 153140
- Author
- oli...@apple.com
- Date
- 2013-07-24 20:59:22 -0700 (Wed, 24 Jul 2013)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (153139 => 153140)
--- trunk/Source/_javascript_Core/ChangeLog 2013-07-25 03:59:20 UTC (rev 153139)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-07-25 03:59:22 UTC (rev 153140)
@@ -1,3 +1,38 @@
+2013-05-01 Filip Pizlo <fpi...@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 <fpi...@apple.com>
fourthTier: Structure::getConcurrently() may be called from for uncacheable dictionaries, and this is safe
Modified: trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp (153139 => 153140)
--- trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2013-07-25 03:59:20 UTC (rev 153139)
+++ trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2013-07-25 03:59:22 UTC (rev 153140)
@@ -210,7 +210,7 @@
// - 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());
Modified: trunk/Source/_javascript_Core/runtime/InitializeThreading.cpp (153139 => 153140)
--- trunk/Source/_javascript_Core/runtime/InitializeThreading.cpp 2013-07-25 03:59:20 UTC (rev 153139)
+++ trunk/Source/_javascript_Core/runtime/InitializeThreading.cpp 2013-07-25 03:59:22 UTC (rev 153140)
@@ -73,7 +73,7 @@
#if ENABLE(FTL_JIT)
LLVMLinkInMCJIT();
LLVMInitializeNativeTarget();
- LLVMInitializeAllAsmPrinters();
+ LLVMInitializeX86AsmPrinter();
#endif
}
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (153139 => 153140)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2013-07-25 03:59:20 UTC (rev 153139)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2013-07-25 03:59:22 UTC (rev 153140)
@@ -321,7 +321,7 @@
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 @@
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 @@
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 @@
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;
}
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (153139 => 153140)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2013-07-25 03:59:20 UTC (rev 153139)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2013-07-25 03:59:22 UTC (rev 153140)
@@ -97,6 +97,7 @@
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 @@
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
