- Revision
- 213175
- Author
- mark....@apple.com
- Date
- 2017-02-28 13:56:44 -0800 (Tue, 28 Feb 2017)
Log Message
Remove setExclusiveThread() and peers from the JSLock.
https://bugs.webkit.org/show_bug.cgi?id=168977
Reviewed by Filip Pizlo.
Source/_javascript_Core:
JSLock::setExclusiveThread() was only used by WebCore. Benchmarking with
Speedometer, we see that removal of exclusive thread status has no measurable
impact on performance. So, let's remove the code for handling exclusive thread
status, and simplify the JSLock code.
For the records, exclusive thread status does improve JSLock locking/unlocking
time by up to 20%. However, this difference is not measurable in the way WebCore
uses the JSLock as confirmed by Speedometer.
Also applied a minor optimization in JSLock::lock() to assume the initial lock
entry case (as opposed to the re-entry case). This appears to shows a small
fractional improvement (about 5%) in JSLock cumulative locking and unlocking
time in a micro-benchmark.
* heap/Heap.cpp:
(JSC::Heap::Heap):
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::MachineThreads):
(JSC::MachineThreads::addCurrentThread):
* heap/MachineStackMarker.h:
* runtime/JSLock.cpp:
(JSC::JSLock::JSLock):
(JSC::JSLock::lock):
(JSC::JSLock::unlock):
(JSC::JSLock::currentThreadIsHoldingLock):
(JSC::JSLock::dropAllLocks):
(JSC::JSLock::grabAllLocks):
(JSC::JSLock::setExclusiveThread): Deleted.
* runtime/JSLock.h:
(JSC::JSLock::ownerThread):
(JSC::JSLock::hasExclusiveThread): Deleted.
(JSC::JSLock::exclusiveThread): Deleted.
* runtime/VM.h:
(JSC::VM::hasExclusiveThread): Deleted.
(JSC::VM::exclusiveThread): Deleted.
(JSC::VM::setExclusiveThread): Deleted.
Source/WebCore:
No new tests because this should already be covered by existing tests.
* bindings/js/CommonVM.cpp:
(WebCore::commonVMSlow):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (213174 => 213175)
--- trunk/Source/_javascript_Core/ChangeLog 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-02-28 21:56:44 UTC (rev 213175)
@@ -1,3 +1,47 @@
+2017-02-28 Mark Lam <mark....@apple.com>
+
+ Remove setExclusiveThread() and peers from the JSLock.
+ https://bugs.webkit.org/show_bug.cgi?id=168977
+
+ Reviewed by Filip Pizlo.
+
+ JSLock::setExclusiveThread() was only used by WebCore. Benchmarking with
+ Speedometer, we see that removal of exclusive thread status has no measurable
+ impact on performance. So, let's remove the code for handling exclusive thread
+ status, and simplify the JSLock code.
+
+ For the records, exclusive thread status does improve JSLock locking/unlocking
+ time by up to 20%. However, this difference is not measurable in the way WebCore
+ uses the JSLock as confirmed by Speedometer.
+
+ Also applied a minor optimization in JSLock::lock() to assume the initial lock
+ entry case (as opposed to the re-entry case). This appears to shows a small
+ fractional improvement (about 5%) in JSLock cumulative locking and unlocking
+ time in a micro-benchmark.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::Heap):
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::MachineThreads):
+ (JSC::MachineThreads::addCurrentThread):
+ * heap/MachineStackMarker.h:
+ * runtime/JSLock.cpp:
+ (JSC::JSLock::JSLock):
+ (JSC::JSLock::lock):
+ (JSC::JSLock::unlock):
+ (JSC::JSLock::currentThreadIsHoldingLock):
+ (JSC::JSLock::dropAllLocks):
+ (JSC::JSLock::grabAllLocks):
+ (JSC::JSLock::setExclusiveThread): Deleted.
+ * runtime/JSLock.h:
+ (JSC::JSLock::ownerThread):
+ (JSC::JSLock::hasExclusiveThread): Deleted.
+ (JSC::JSLock::exclusiveThread): Deleted.
+ * runtime/VM.h:
+ (JSC::VM::hasExclusiveThread): Deleted.
+ (JSC::VM::exclusiveThread): Deleted.
+ (JSC::VM::setExclusiveThread): Deleted.
+
2017-02-28 Saam Barati <sbar...@apple.com>
Arm64 disassembler prints "ars" instead of "asr"
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (213174 => 213175)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2017-02-28 21:56:44 UTC (rev 213175)
@@ -258,7 +258,7 @@
, m_objectSpace(this)
, m_extraMemorySize(0)
, m_deprecatedExtraMemorySize(0)
- , m_machineThreads(std::make_unique<MachineThreads>(this))
+ , m_machineThreads(std::make_unique<MachineThreads>())
, m_collectorSlotVisitor(std::make_unique<SlotVisitor>(*this, "C"))
, m_mutatorSlotVisitor(std::make_unique<SlotVisitor>(*this, "M"))
, m_mutatorMarkStack(std::make_unique<MarkStackArray>())
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (213174 => 213175)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2017-02-28 21:56:44 UTC (rev 213175)
@@ -190,14 +190,10 @@
#endif
}
-MachineThreads::MachineThreads(Heap* heap)
+MachineThreads::MachineThreads()
: m_registeredThreads(0)
, m_threadSpecificForMachineThreads(0)
-#if !ASSERT_DISABLED
- , m_heap(heap)
-#endif
{
- UNUSED_PARAM(heap);
threadSpecificKeyCreate(&m_threadSpecificForMachineThreads, removeThread);
activeMachineThreadsManager().add(this);
}
@@ -234,8 +230,6 @@
void MachineThreads::addCurrentThread()
{
- ASSERT(!m_heap->vm()->hasExclusiveThread() || m_heap->vm()->exclusiveThread() == std::this_thread::get_id());
-
if (threadSpecificGet(m_threadSpecificForMachineThreads)) {
#ifndef NDEBUG
LockHolder lock(m_registeredThreadsMutex);
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.h (213174 => 213175)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2017-02-28 21:56:44 UTC (rev 213175)
@@ -67,7 +67,7 @@
class MachineThreads {
WTF_MAKE_NONCOPYABLE(MachineThreads);
public:
- MachineThreads(Heap*);
+ MachineThreads();
~MachineThreads();
void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, CurrentThreadState*);
@@ -163,9 +163,6 @@
Lock m_registeredThreadsMutex;
Thread* m_registeredThreads;
WTF::ThreadSpecificKey m_threadSpecificForMachineThreads;
-#if !ASSERT_DISABLED
- Heap* m_heap;
-#endif
};
#define DECLARE_AND_COMPUTE_CURRENT_THREAD_STATE(stateName) \
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (213174 => 213175)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2017-02-28 21:56:44 UTC (rev 213175)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2005, 2008, 2012, 2014, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2017 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -78,7 +78,6 @@
: m_ownerThreadID(std::thread::id())
, m_lockCount(0)
, m_lockDropDepth(0)
- , m_hasExclusiveThread(false)
, m_vm(vm)
, m_entryAtomicStringTable(nullptr)
{
@@ -94,13 +93,6 @@
m_vm = nullptr;
}
-void JSLock::setExclusiveThread(std::thread::id threadId)
-{
- RELEASE_ASSERT(!m_lockCount && m_ownerThreadID == std::thread::id());
- m_hasExclusiveThread = (threadId != std::thread::id());
- m_ownerThreadID = threadId;
-}
-
void JSLock::lock()
{
lock(1);
@@ -109,15 +101,16 @@
void JSLock::lock(intptr_t lockCount)
{
ASSERT(lockCount > 0);
- if (currentThreadIsHoldingLock()) {
- m_lockCount += lockCount;
- return;
+ bool success = m_lock.tryLock();
+ if (UNLIKELY(!success)) {
+ if (currentThreadIsHoldingLock()) {
+ m_lockCount += lockCount;
+ return;
+ }
+ m_lock.lock();
}
- if (!m_hasExclusiveThread) {
- m_lock.lock();
- m_ownerThreadID = std::this_thread::get_id();
- }
+ m_ownerThreadID = std::this_thread::get_id();
ASSERT(!m_lockCount);
m_lockCount = lockCount;
@@ -175,11 +168,8 @@
m_lockCount -= unlockCount;
if (!m_lockCount) {
-
- if (!m_hasExclusiveThread) {
- m_ownerThreadID = std::thread::id();
- m_lock.unlock();
- }
+ m_ownerThreadID = std::thread::id();
+ m_lock.unlock();
}
}
@@ -214,9 +204,6 @@
bool JSLock::currentThreadIsHoldingLock()
{
- ASSERT(!m_hasExclusiveThread || (exclusiveThread() == std::this_thread::get_id()));
- if (m_hasExclusiveThread)
- return !!m_lockCount;
return m_ownerThreadID == std::this_thread::get_id();
}
@@ -223,11 +210,6 @@
// This function returns the number of locks that were dropped.
unsigned JSLock::dropAllLocks(DropAllLocks* dropper)
{
- if (m_hasExclusiveThread) {
- ASSERT(exclusiveThread() == std::this_thread::get_id());
- return 0;
- }
-
if (!currentThreadIsHoldingLock())
return 0;
@@ -247,8 +229,6 @@
void JSLock::grabAllLocks(DropAllLocks* dropper, unsigned droppedLockCount)
{
- ASSERT(!m_hasExclusiveThread || !droppedLockCount);
-
// If no locks were dropped, nothing to do!
if (!droppedLockCount)
return;
Modified: trunk/Source/_javascript_Core/runtime/JSLock.h (213174 => 213175)
--- trunk/Source/_javascript_Core/runtime/JSLock.h 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/runtime/JSLock.h 2017-02-28 21:56:44 UTC (rev 213175)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2005, 2008, 2009, 2014, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2017 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -93,14 +93,7 @@
VM* vm() { return m_vm; }
- bool hasExclusiveThread() const { return m_hasExclusiveThread; }
- std::thread::id exclusiveThread() const
- {
- ASSERT(m_hasExclusiveThread);
- return m_ownerThreadID;
- }
std::thread::id ownerThread() const { return m_ownerThreadID; }
- JS_EXPORT_PRIVATE void setExclusiveThread(std::thread::id);
JS_EXPORT_PRIVATE bool currentThreadIsHoldingLock();
void willDestroyVM(VM*);
@@ -136,7 +129,6 @@
std::thread::id m_ownerThreadID;
intptr_t m_lockCount;
unsigned m_lockDropDepth;
- bool m_hasExclusiveThread;
bool m_shouldReleaseHeapAccess;
VM* m_vm;
AtomicStringTable* m_entryAtomicStringTable;
Modified: trunk/Source/_javascript_Core/runtime/VM.h (213174 => 213175)
--- trunk/Source/_javascript_Core/runtime/VM.h 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2017-02-28 21:56:44 UTC (rev 213175)
@@ -611,10 +611,6 @@
RTTraceList* m_rtTraceList;
#endif
- bool hasExclusiveThread() const { return m_apiLock->hasExclusiveThread(); }
- std::thread::id exclusiveThread() const { return m_apiLock->exclusiveThread(); }
- void setExclusiveThread(std::thread::id threadId) { m_apiLock->setExclusiveThread(threadId); }
-
std::thread::id ownerThread() const { return m_apiLock->ownerThread(); }
JS_EXPORT_PRIVATE void resetDateCache();
Modified: trunk/Source/WebCore/ChangeLog (213174 => 213175)
--- trunk/Source/WebCore/ChangeLog 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/WebCore/ChangeLog 2017-02-28 21:56:44 UTC (rev 213175)
@@ -1,3 +1,15 @@
+2017-02-28 Mark Lam <mark....@apple.com>
+
+ Remove setExclusiveThread() and peers from the JSLock.
+ https://bugs.webkit.org/show_bug.cgi?id=168977
+
+ Reviewed by Filip Pizlo.
+
+ No new tests because this should already be covered by existing tests.
+
+ * bindings/js/CommonVM.cpp:
+ (WebCore::commonVMSlow):
+
2017-02-28 Dave Hyatt <hy...@apple.com>
Centering text inside a button set to display flex and justify-content: center is impossible
Modified: trunk/Source/WebCore/bindings/js/CommonVM.cpp (213174 => 213175)
--- trunk/Source/WebCore/bindings/js/CommonVM.cpp 2017-02-28 21:54:38 UTC (rev 213174)
+++ trunk/Source/WebCore/bindings/js/CommonVM.cpp 2017-02-28 21:56:44 UTC (rev 213175)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -53,9 +53,7 @@
ScriptController::initializeThreading();
g_commonVMOrNull = &VM::createLeaked(LargeHeap).leakRef();
g_commonVMOrNull->heap.acquireAccess(); // At any time, we may do things that affect the GC.
-#if !PLATFORM(IOS)
- g_commonVMOrNull->setExclusiveThread(std::this_thread::get_id());
-#else
+#if PLATFORM(IOS)
g_commonVMOrNull->heap.setRunLoop(WebThreadRunLoop());
g_commonVMOrNull->heap.machineThreads().addCurrentThread();
#endif
