- Revision
- 270794
- Author
- tzaga...@apple.com
- Date
- 2020-12-14 11:47:36 -0800 (Mon, 14 Dec 2020)
Log Message
Move some of the work from JSLock to VMEntryScope
https://bugs.webkit.org/show_bug.cgi?id=219830
Reviewed by Mark Lam.
Source/_javascript_Core:
We move several things from JSLock to VMEntryScope that could only be observed after we entered the VM:
- WasmThreads: only used when tiering up wasm, so VMEntryScope would have executed
- registerThreadForMachExceptionHandling: The mach exception handlers are used for:
- sigill crash analyzer: only relevant after we enter the vm
- wasm fault signal handler: same, we must be executing wasm and therefore VMEntryScope will have executed.
- VMTraps: only handles faults in JIT code
- firePrimitiveGigacageEnabledIfNecessary: Only watched by the JITs
This gives is a ~10% improvement on APIBench (score change from ~36.3 to ~39.9), but as it turns out the most expensive
call is adding the current thread to the heap as this requires acquiring two locks. We can't move this to VMEntryScope
since it's possible to use the API and GC without ever entering the VM, which would result in the current thread's stack
not being scanned. Instead, we just remember the last thread that acquired the lock and skip the call if we're seeing the
same thread again. This greatly amortizes the cost and gives us another ~10%:
CURRENT_API: Baseline Change
----------------------------------------
RichardsMostlyC: 62ms 32ms
RichardsMostlyObjC: 303ms 264ms
RichardsMostlySwift: 296ms 261ms
RichardsSomeC: 76ms 49ms
RichardsSomeObjC: 156ms 150ms
RichardsSomeSwift: 200ms 197ms
UPCOMING_API: Baseline Change
----------------------------------------
RichardsMostlyC: 19ms 19ms
RichardsMostlyObjC: 282ms 260ms
RichardsMostlySwift: 282ms 264ms
RichardsSomeC: 79ms 46ms
RichardsSomeObjC: 156ms 149ms
RichardsSomeSwift: 195ms 195ms
----------------------------------------
Score: 36.2211 43.3368
* runtime/JSLock.cpp:
(JSC::JSLock::didAcquireLock):
(JSC::JSLock::willReleaseLock):
* runtime/JSLock.h:
* runtime/VMEntryScope.cpp:
(JSC::VMEntryScope::VMEntryScope):
(JSC::VMEntryScope::~VMEntryScope):
Source/WTF:
Add a unique ID to threads. This uid is used in JSLock to avoid unnecessary work when the
same thread that last acquired the lock is reacquiring it.
* wtf/Threading.cpp:
* wtf/Threading.h:
(WTF::Thread::uid const):
(WTF::Thread::Thread):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (270793 => 270794)
--- trunk/Source/_javascript_Core/ChangeLog 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-12-14 19:47:36 UTC (rev 270794)
@@ -1,3 +1,52 @@
+2020-12-14 Tadeu Zagallo <tzaga...@apple.com>
+
+ Move some of the work from JSLock to VMEntryScope
+ https://bugs.webkit.org/show_bug.cgi?id=219830
+
+ Reviewed by Mark Lam.
+
+ We move several things from JSLock to VMEntryScope that could only be observed after we entered the VM:
+ - WasmThreads: only used when tiering up wasm, so VMEntryScope would have executed
+ - registerThreadForMachExceptionHandling: The mach exception handlers are used for:
+ - sigill crash analyzer: only relevant after we enter the vm
+ - wasm fault signal handler: same, we must be executing wasm and therefore VMEntryScope will have executed.
+ - VMTraps: only handles faults in JIT code
+ - firePrimitiveGigacageEnabledIfNecessary: Only watched by the JITs
+
+ This gives is a ~10% improvement on APIBench (score change from ~36.3 to ~39.9), but as it turns out the most expensive
+ call is adding the current thread to the heap as this requires acquiring two locks. We can't move this to VMEntryScope
+ since it's possible to use the API and GC without ever entering the VM, which would result in the current thread's stack
+ not being scanned. Instead, we just remember the last thread that acquired the lock and skip the call if we're seeing the
+ same thread again. This greatly amortizes the cost and gives us another ~10%:
+
+ CURRENT_API: Baseline Change
+ ----------------------------------------
+ RichardsMostlyC: 62ms 32ms
+ RichardsMostlyObjC: 303ms 264ms
+ RichardsMostlySwift: 296ms 261ms
+ RichardsSomeC: 76ms 49ms
+ RichardsSomeObjC: 156ms 150ms
+ RichardsSomeSwift: 200ms 197ms
+
+ UPCOMING_API: Baseline Change
+ ----------------------------------------
+ RichardsMostlyC: 19ms 19ms
+ RichardsMostlyObjC: 282ms 260ms
+ RichardsMostlySwift: 282ms 264ms
+ RichardsSomeC: 79ms 46ms
+ RichardsSomeObjC: 156ms 149ms
+ RichardsSomeSwift: 195ms 195ms
+ ----------------------------------------
+ Score: 36.2211 43.3368
+
+ * runtime/JSLock.cpp:
+ (JSC::JSLock::didAcquireLock):
+ (JSC::JSLock::willReleaseLock):
+ * runtime/JSLock.h:
+ * runtime/VMEntryScope.cpp:
+ (JSC::VMEntryScope::VMEntryScope):
+ (JSC::VMEntryScope::~VMEntryScope):
+
2020-12-14 Robin Morisset <rmoris...@apple.com>
Minor cleanup of BigInts
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (270793 => 270794)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2020-12-14 19:47:36 UTC (rev 270794)
@@ -25,8 +25,6 @@
#include "JSGlobalObject.h"
#include "MachineStackMarker.h"
#include "SamplingProfiler.h"
-#include "WasmCapabilities.h"
-#include "WasmMachineThreads.h"
#include <wtf/StackPointer.h>
#include <wtf/Threading.h>
#include <wtf/threads/Signals.h>
@@ -148,28 +146,23 @@
void* p = currentStackPointer();
m_vm->setStackPointerAtVMEntry(p);
- if (m_vm->heap.machineThreads().addCurrentThread()) {
- if (isKernTCSMAvailable())
- enableKernTCSM();
+ if (thread.uid() != m_lastOwnerThread) {
+ m_lastOwnerThread = thread.uid();
+ if (m_vm->heap.machineThreads().addCurrentThread()) {
+ if (isKernTCSMAvailable())
+ enableKernTCSM();
+ }
}
-#if ENABLE(WEBASSEMBLY)
- if (Wasm::isSupported())
- Wasm::startTrackingCurrentThread();
-#endif
-
-#if HAVE(MACH_EXCEPTIONS)
- registerThreadForMachExceptionHandling(Thread::current());
-#endif
-
// Note: everything below must come after addCurrentThread().
m_vm->traps().notifyGrabAllLocks();
-
- m_vm->firePrimitiveGigacageEnabledIfNecessary();
#if ENABLE(SAMPLING_PROFILER)
- if (SamplingProfiler* samplingProfiler = m_vm->samplingProfiler())
- samplingProfiler->noticeJSLockAcquisition();
+ {
+ SamplingProfiler* samplingProfiler = m_vm->samplingProfiler();
+ if (UNLIKELY(samplingProfiler))
+ samplingProfiler->noticeJSLockAcquisition();
+ }
#endif
}
Modified: trunk/Source/_javascript_Core/runtime/JSLock.h (270793 => 270794)
--- trunk/Source/_javascript_Core/runtime/JSLock.h 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/_javascript_Core/runtime/JSLock.h 2020-12-14 19:47:36 UTC (rev 270794)
@@ -145,10 +145,11 @@
// different thread, and an optional is vulnerable to races.
// See https://bugs.webkit.org/show_bug.cgi?id=169042#c6
bool m_hasOwnerThread { false };
+ bool m_shouldReleaseHeapAccess;
RefPtr<Thread> m_ownerThread;
intptr_t m_lockCount;
unsigned m_lockDropDepth;
- bool m_shouldReleaseHeapAccess;
+ uint32_t m_lastOwnerThread { 0 };
VM* m_vm;
AtomStringTable* m_entryAtomStringTable;
};
Modified: trunk/Source/_javascript_Core/runtime/VMEntryScope.cpp (270793 => 270794)
--- trunk/Source/_javascript_Core/runtime/VMEntryScope.cpp 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/_javascript_Core/runtime/VMEntryScope.cpp 2020-12-14 19:47:36 UTC (rev 270794)
@@ -29,6 +29,8 @@
#include "Options.h"
#include "SamplingProfiler.h"
#include "VM.h"
+#include "WasmCapabilities.h"
+#include "WasmMachineThreads.h"
#include "Watchdog.h"
#include <wtf/SystemTracing.h>
@@ -41,6 +43,17 @@
if (!vm.entryScope) {
vm.entryScope = this;
+#if ENABLE(WEBASSEMBLY)
+ if (Wasm::isSupported())
+ Wasm::startTrackingCurrentThread();
+#endif
+
+#if HAVE(MACH_EXCEPTIONS)
+ registerThreadForMachExceptionHandling(Thread::current());
+#endif
+
+ vm.firePrimitiveGigacageEnabledIfNecessary();
+
// Reset the date cache between JS invocations to force the VM to
// observe time zone changes.
// FIXME: We should clear it only when we know the timezone has been changed.
@@ -51,10 +64,13 @@
vm.watchdog()->enteredVM();
#if ENABLE(SAMPLING_PROFILER)
- if (SamplingProfiler* samplingProfiler = vm.samplingProfiler())
- samplingProfiler->noticeVMEntry();
+ {
+ SamplingProfiler* samplingProfiler = vm.samplingProfiler();
+ if (UNLIKELY(samplingProfiler))
+ samplingProfiler->noticeVMEntry();
+ }
#endif
- if (Options::useTracePoints())
+ if (UNLIKELY(Options::useTracePoints()))
tracePoint(VMEntryScopeStart);
}
Modified: trunk/Source/WTF/ChangeLog (270793 => 270794)
--- trunk/Source/WTF/ChangeLog 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/WTF/ChangeLog 2020-12-14 19:47:36 UTC (rev 270794)
@@ -1,3 +1,18 @@
+2020-12-14 Tadeu Zagallo <tzaga...@apple.com>
+
+ Move some of the work from JSLock to VMEntryScope
+ https://bugs.webkit.org/show_bug.cgi?id=219830
+
+ Reviewed by Mark Lam.
+
+ Add a unique ID to threads. This uid is used in JSLock to avoid unnecessary work when the
+ same thread that last acquired the lock is reacquiring it.
+
+ * wtf/Threading.cpp:
+ * wtf/Threading.h:
+ (WTF::Thread::uid const):
+ (WTF::Thread::Thread):
+
2020-12-13 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Introduce vmEntryCustomAccessor and vmEntryHostFunction for JITCage
Modified: trunk/Source/WTF/wtf/Threading.cpp (270793 => 270794)
--- trunk/Source/WTF/wtf/Threading.cpp 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/WTF/wtf/Threading.cpp 2020-12-14 19:47:36 UTC (rev 270794)
@@ -64,6 +64,8 @@
#endif
}
+std::atomic<uint32_t> Thread::s_uid { 0 };
+
struct Thread::NewThreadContext : public ThreadSafeRefCounted<NewThreadContext> {
public:
NewThreadContext(const char* name, Function<void()>&& entryPoint, Ref<Thread>&& thread)
Modified: trunk/Source/WTF/wtf/Threading.h (270793 => 270794)
--- trunk/Source/WTF/wtf/Threading.h 2020-12-14 19:39:53 UTC (rev 270793)
+++ trunk/Source/WTF/wtf/Threading.h 2020-12-14 19:47:36 UTC (rev 270794)
@@ -92,6 +92,7 @@
};
class Thread : public ThreadSafeRefCounted<Thread> {
+ static std::atomic<uint32_t> s_uid;
public:
friend class ThreadGroup;
friend WTF_EXPORT_PRIVATE void initialize();
@@ -111,6 +112,8 @@
WTF_EXPORT_PRIVATE unsigned numberOfThreadGroups();
+ uint32_t uid() const { return m_uid; }
+
#if OS(WINDOWS)
// Returns ThreadIdentifier directly. It is useful if the user only cares about identity
// of threads. At that time, users should know that holding this ThreadIdentifier does not ensure
@@ -328,6 +331,7 @@
StackBounds m_stack { StackBounds::emptyBounds() };
HashMap<ThreadGroup*, std::weak_ptr<ThreadGroup>> m_threadGroupMap;
PlatformThreadHandle m_handle;
+ uint32_t m_uid;
#if OS(WINDOWS)
ThreadIdentifier m_id { 0 };
#elif OS(DARWIN)
@@ -362,6 +366,7 @@
, m_isDestroyedOnce(false)
, m_isCompilationThread(false)
, m_gcThreadType(static_cast<unsigned>(GCThreadType::None))
+ , m_uid(++s_uid)
{
}