From 2664a259a96c10b4ed711351edee9e6a307912b4 Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Wed, 20 Feb 2019 12:27:54 +0100 Subject: [PATCH] [wasm] Use shared mutex for background compile token This introduces a new {base::SharedMutex}, which mimics {std::shared_mutex}, available in C++17. It is being used for synchronizing the WebAssembly background compile tasks. This removes a lot of unnecessary contention, leading to synchronization of background tasks that should be able to run in parallel ideally. Locally, this reduces Liftoff compilation time by more than 20 percent. R=mstarzinger@chromium.org, mlippautz@chromium.org Bug: chromium:924956 Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel Change-Id: I8c522faf385149bfe2cf00d777a7942c537f9cd2 Reviewed-on: https://chromium-review.googlesource.com/c/1477731 Commit-Queue: Clemens Hammacher Reviewed-by: Michael Lippautz Reviewed-by: Michael Starzinger Cr-Commit-Position: refs/heads/master@{#59729} --- src/base/platform/mutex.cc | 61 +++++++++++++++++++++ src/base/platform/mutex.h | 104 +++++++++++++++++++++++++++++++++++- src/wasm/module-compiler.cc | 36 +++++++------ 3 files changed, 183 insertions(+), 18 deletions(-) diff --git a/src/base/platform/mutex.cc b/src/base/platform/mutex.cc index a044075c16..2e2f7f9320 100644 --- a/src/base/platform/mutex.cc +++ b/src/base/platform/mutex.cc @@ -155,6 +155,45 @@ bool RecursiveMutex::TryLock() { return true; } +SharedMutex::SharedMutex() { pthread_rwlock_init(&native_handle_, nullptr); } + +SharedMutex::~SharedMutex() { + int result = pthread_rwlock_destroy(&native_handle_); + DCHECK_EQ(0, result); + USE(result); +} + +void SharedMutex::LockShared() { + int result = pthread_rwlock_rdlock(&native_handle_); + DCHECK_EQ(0, result); + USE(result); +} + +void SharedMutex::LockExclusive() { + int result = pthread_rwlock_wrlock(&native_handle_); + DCHECK_EQ(0, result); + USE(result); +} + +void SharedMutex::UnlockShared() { + int result = pthread_rwlock_unlock(&native_handle_); + DCHECK_EQ(0, result); + USE(result); +} + +void SharedMutex::UnlockExclusive() { + // Same code as {UnlockShared} on POSIX. + UnlockShared(); +} + +bool SharedMutex::TryLockShared() { + return pthread_rwlock_tryrdlock(&native_handle_) == 0; +} + +bool SharedMutex::TryLockExclusive() { + return pthread_rwlock_trywrlock(&native_handle_) == 0; +} + #elif V8_OS_WIN Mutex::Mutex() : native_handle_(SRWLOCK_INIT) { @@ -233,6 +272,28 @@ bool RecursiveMutex::TryLock() { return true; } +SharedMutex::SharedMutex() : native_handle_(SRWLOCK_INIT) {} + +SharedMutex::~SharedMutex() {} + +void SharedMutex::LockShared() { AcquireSRWLockShared(&native_handle_); } + +void SharedMutex::LockExclusive() { AcquireSRWLockExclusive(&native_handle_); } + +void SharedMutex::UnlockShared() { ReleaseSRWLockShared(&native_handle_); } + +void SharedMutex::UnlockExclusive() { + ReleaseSRWLockExclusive(&native_handle_); +} + +bool SharedMutex::TryLockShared() { + return TryAcquireSRWLockShared(&native_handle_); +} + +bool SharedMutex::TryLockExclusive() { + return TryAcquireSRWLockExclusive(&native_handle_); +} + #endif // V8_OS_POSIX } // namespace base diff --git a/src/base/platform/mutex.h b/src/base/platform/mutex.h index 4ee0deb2cf..ea589d5b98 100644 --- a/src/base/platform/mutex.h +++ b/src/base/platform/mutex.h @@ -150,6 +150,7 @@ class V8_BASE_EXPORT RecursiveMutex final { // successfully locked. bool TryLock() V8_WARN_UNUSED_RESULT; + private: // The implementation-defined native handle type. #if V8_OS_POSIX typedef pthread_mutex_t NativeHandle; @@ -157,7 +158,6 @@ class V8_BASE_EXPORT RecursiveMutex final { typedef CRITICAL_SECTION NativeHandle; #endif - private: NativeHandle native_handle_; #ifdef DEBUG int level_; @@ -183,6 +183,73 @@ typedef LazyStaticInstance; +enum MutexSharedType : bool { kShared = true, kExclusive = false }; + +template +class SharedMutexGuard final { + public: + explicit SharedMutexGuard(SharedMutex* mutex) : mutex_(mutex) { + if (!has_mutex()) return; + if (kIsShared) { + mutex_->LockShared(); + } else { + mutex_->LockExclusive(); + } + } + ~SharedMutexGuard() { + if (!has_mutex()) return; + if (kIsShared) { + mutex_->UnlockShared(); + } else { + mutex_->UnlockExclusive(); + } + } + + private: + SharedMutex* const mutex_; + + bool V8_INLINE has_mutex() const { + DCHECK_IMPLIES(Behavior == NullBehavior::kRequireNotNull, + mutex_ != nullptr); + return Behavior == NullBehavior::kRequireNotNull || mutex_ != nullptr; + } + + DISALLOW_COPY_AND_ASSIGN(SharedMutexGuard); +}; + } // namespace base } // namespace v8 diff --git a/src/wasm/module-compiler.cc b/src/wasm/module-compiler.cc index 29eba9f10e..c2f00661b8 100644 --- a/src/wasm/module-compiler.cc +++ b/src/wasm/module-compiler.cc @@ -65,24 +65,21 @@ class BackgroundCompileToken { : native_module_(native_module) {} void Cancel() { - base::MutexGuard mutex_guard(&mutex_); + base::SharedMutexGuard mutex_guard(&mutex_); native_module_ = nullptr; } - // Only call this while holding the {mutex_}. - void CancelLocked() { native_module_ = nullptr; } - private: friend class BackgroundCompileScope; - base::Mutex mutex_; + base::SharedMutex mutex_; NativeModule* native_module_; NativeModule* StartScope() { - mutex_.Lock(); + mutex_.LockShared(); return native_module_; } - void ExitScope() { mutex_.Unlock(); } + void ExitScope() { mutex_.UnlockShared(); } }; class CompilationStateImpl; @@ -681,6 +678,7 @@ class BackgroundCompileTask : public CancelableTask { &env.value(), wire_bytes, async_counters_.get(), &detected_features); // Step 3 (synchronized): Publish the compilation result. + bool cancel_compilation = false; { BackgroundCompileScope compile_scope(token_); if (compile_scope.cancelled()) return; @@ -690,18 +688,22 @@ class BackgroundCompileTask : public CancelableTask { compile_scope.compilation_state()->OnBackgroundTaskStopped( detected_features); // Also, cancel all remaining compilation. - token_->CancelLocked(); - return; - } - compile_scope.compilation_state()->OnFinishedUnit( - unit->requested_tier(), code); - if (deadline < MonotonicallyIncreasingTimeInMs()) { - compile_scope.compilation_state()->ReportDetectedFeatures( - detected_features); - compile_scope.compilation_state()->RestartBackgroundCompileTask(); - return; + cancel_compilation = true; + } else { + compile_scope.compilation_state()->OnFinishedUnit( + unit->requested_tier(), code); + if (deadline < MonotonicallyIncreasingTimeInMs()) { + compile_scope.compilation_state()->ReportDetectedFeatures( + detected_features); + compile_scope.compilation_state()->RestartBackgroundCompileTask(); + return; + } } } + if (cancel_compilation) { + token_->Cancel(); + return; + } } UNREACHABLE(); // Loop exits via explicit return. }