- Revision
- 179609
- Author
- [email protected]
- Date
- 2015-02-04 10:00:05 -0800 (Wed, 04 Feb 2015)
Log Message
Remove concept of makeUsableFromMultipleThreads().
<https://webkit.org/b/141221>
Reviewed by Mark Hahnenberg.
Source/_javascript_Core:
Currently, we rely on VM::makeUsableFromMultipleThreads() being called before we
start acquiring the JSLock and entering the VM from different threads.
Acquisition of the JSLock will register the acquiring thread with the VM's thread
registry if not already registered. However, it will only do this if the VM's
thread specific key has been initialized by makeUsableFromMultipleThreads().
This is fragile, and also does not read intuitively because one would expect to
acquire the JSLock before calling any methods on the VM. This is exactly what
JSGlobalContextCreateInGroup() did (i.e. acquire the lock before calling
makeUsableFromMultipleThreads()), but is wrong. The result is that the invoking
thread will not have been registered with the VM during that first entry into
the VM.
The fix is to make it so that we initialize the VM's thread specific key on
construction of the VM's MachineThreads registry instead of relying on
makeUsableFromMultipleThreads() being called. With this, we can eliminate
makeUsableFromMultipleThreads() altogether.
Performance results are neutral in aggregate.
* API/JSContextRef.cpp:
(JSGlobalContextCreateInGroup):
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::MachineThreads):
(JSC::MachineThreads::~MachineThreads):
(JSC::MachineThreads::addCurrentThread):
(JSC::MachineThreads::removeThread):
(JSC::MachineThreads::gatherConservativeRoots):
(JSC::MachineThreads::makeUsableFromMultipleThreads): Deleted.
* heap/MachineStackMarker.h:
* runtime/VM.cpp:
(JSC::VM::sharedInstance):
* runtime/VM.h:
(JSC::VM::makeUsableFromMultipleThreads): Deleted.
Source/WebCore:
No new tests.
* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::commonVM):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/JSContextRef.cpp (179608 => 179609)
--- trunk/Source/_javascript_Core/API/JSContextRef.cpp 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/_javascript_Core/API/JSContextRef.cpp 2015-02-04 18:00:05 UTC (rev 179609)
@@ -138,7 +138,6 @@
RefPtr<VM> vm = group ? PassRefPtr<VM>(toJS(group)) : VM::createContextGroup();
JSLockHolder locker(vm.get());
- vm->makeUsableFromMultipleThreads();
if (!globalObjectClass) {
JSGlobalObject* globalObject = JSGlobalObject::create(*vm, JSGlobalObject::createStructure(*vm, jsNull()));
Modified: trunk/Source/_javascript_Core/ChangeLog (179608 => 179609)
--- trunk/Source/_javascript_Core/ChangeLog 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-02-04 18:00:05 UTC (rev 179609)
@@ -1,3 +1,45 @@
+2015-02-04 Mark Lam <[email protected]>
+
+ Remove concept of makeUsableFromMultipleThreads().
+ <https://webkit.org/b/141221>
+
+ Reviewed by Mark Hahnenberg.
+
+ Currently, we rely on VM::makeUsableFromMultipleThreads() being called before we
+ start acquiring the JSLock and entering the VM from different threads.
+ Acquisition of the JSLock will register the acquiring thread with the VM's thread
+ registry if not already registered. However, it will only do this if the VM's
+ thread specific key has been initialized by makeUsableFromMultipleThreads().
+
+ This is fragile, and also does not read intuitively because one would expect to
+ acquire the JSLock before calling any methods on the VM. This is exactly what
+ JSGlobalContextCreateInGroup() did (i.e. acquire the lock before calling
+ makeUsableFromMultipleThreads()), but is wrong. The result is that the invoking
+ thread will not have been registered with the VM during that first entry into
+ the VM.
+
+ The fix is to make it so that we initialize the VM's thread specific key on
+ construction of the VM's MachineThreads registry instead of relying on
+ makeUsableFromMultipleThreads() being called. With this, we can eliminate
+ makeUsableFromMultipleThreads() altogether.
+
+ Performance results are neutral in aggregate.
+
+ * API/JSContextRef.cpp:
+ (JSGlobalContextCreateInGroup):
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::MachineThreads):
+ (JSC::MachineThreads::~MachineThreads):
+ (JSC::MachineThreads::addCurrentThread):
+ (JSC::MachineThreads::removeThread):
+ (JSC::MachineThreads::gatherConservativeRoots):
+ (JSC::MachineThreads::makeUsableFromMultipleThreads): Deleted.
+ * heap/MachineStackMarker.h:
+ * runtime/VM.cpp:
+ (JSC::VM::sharedInstance):
+ * runtime/VM.h:
+ (JSC::VM::makeUsableFromMultipleThreads): Deleted.
+
2015-02-04 Chris Dumez <[email protected]>
Add removeFirst(value) / removeAll(value) methods to WTF::Vector
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (179608 => 179609)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2015-02-04 18:00:05 UTC (rev 179609)
@@ -123,12 +123,12 @@
#endif
{
UNUSED_PARAM(heap);
+ threadSpecificKeyCreate(&m_threadSpecific, removeThread);
}
MachineThreads::~MachineThreads()
{
- if (m_threadSpecific)
- threadSpecificKeyDelete(m_threadSpecific);
+ threadSpecificKeyDelete(m_threadSpecific);
MutexLocker registeredThreadsLock(m_registeredThreadsMutex);
for (Thread* t = m_registeredThreads; t;) {
@@ -160,20 +160,14 @@
#endif
}
-void MachineThreads::makeUsableFromMultipleThreads()
-{
- if (m_threadSpecific)
- return;
-
- threadSpecificKeyCreate(&m_threadSpecific, removeThread);
-}
-
void MachineThreads::addCurrentThread()
{
ASSERT(!m_heap->vm()->hasExclusiveThread() || m_heap->vm()->exclusiveThread() == std::this_thread::get_id());
- if (!m_threadSpecific || threadSpecificGet(m_threadSpecific))
+ if (threadSpecificGet(m_threadSpecific)) {
+ ASSERT(threadSpecificGet(m_threadSpecific) == this);
return;
+ }
threadSpecificSet(m_threadSpecific, this);
Thread* thread = new Thread(getCurrentPlatformThread(), wtfThreadData().stack().origin());
@@ -186,8 +180,7 @@
void MachineThreads::removeThread(void* p)
{
- if (p)
- static_cast<MachineThreads*>(p)->removeCurrentThread();
+ static_cast<MachineThreads*>(p)->removeCurrentThread();
}
template<typename PlatformThread>
@@ -521,9 +514,6 @@
{
gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, currentThreadRegisters);
- if (!m_threadSpecific)
- return;
-
size_t size;
size_t capacity = 0;
void* buffer = nullptr;
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.h (179608 => 179609)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2015-02-04 18:00:05 UTC (rev 179609)
@@ -44,7 +44,6 @@
void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
- JS_EXPORT_PRIVATE void makeUsableFromMultipleThreads();
JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
private:
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (179608 => 179609)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2015-02-04 18:00:05 UTC (rev 179609)
@@ -357,10 +357,8 @@
{
GlobalJSLock globalLock;
VM*& instance = sharedInstanceInternal();
- if (!instance) {
+ if (!instance)
instance = adoptRef(new VM(APIShared, SmallHeap)).leakRef();
- instance->makeUsableFromMultipleThreads();
- }
return *instance;
}
Modified: trunk/Source/_javascript_Core/runtime/VM.h (179608 => 179609)
--- trunk/Source/_javascript_Core/runtime/VM.h 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2015-02-04 18:00:05 UTC (rev 179609)
@@ -217,8 +217,6 @@
static PassRefPtr<VM> createContextGroup(HeapType = SmallHeap);
JS_EXPORT_PRIVATE ~VM();
- void makeUsableFromMultipleThreads() { heap.machineThreads().makeUsableFromMultipleThreads(); }
-
private:
RefPtr<JSLock> m_apiLock;
Modified: trunk/Source/WebCore/ChangeLog (179608 => 179609)
--- trunk/Source/WebCore/ChangeLog 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/WebCore/ChangeLog 2015-02-04 18:00:05 UTC (rev 179609)
@@ -1,3 +1,15 @@
+2015-02-04 Mark Lam <[email protected]>
+
+ Remove concept of makeUsableFromMultipleThreads().
+ <https://webkit.org/b/141221>
+
+ Reviewed by Mark Hahnenberg.
+
+ No new tests.
+
+ * bindings/js/JSDOMWindowBase.cpp:
+ (WebCore::JSDOMWindowBase::commonVM):
+
2015-02-04 Simon Fraser <[email protected]>
[iOS WK2] Assert in ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren() on tab switching
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp (179608 => 179609)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp 2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp 2015-02-04 18:00:05 UTC (rev 179609)
@@ -212,7 +212,6 @@
vm->heap.setEdenActivityCallback(vm->heap.fullActivityCallback());
#endif
vm->heap.setIncrementalSweeper(std::make_unique<WebSafeIncrementalSweeper>(&vm->heap));
- vm->makeUsableFromMultipleThreads();
vm->heap.machineThreads().addCurrentThread();
#endif
initNormalWorldClientData(vm);
