From cd86d0d20ab167c33b23d3875db68d1d4bad3a3b Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 25 Apr 2019 08:01:58 -0700 Subject: [PATCH] Export of internal Abseil changes. -- 997d2a8d12d9395046b0bdfc2f206a0b2fe2f1f9 by Abseil Team : Typo fix: IsHashCallble -> IsHashCallable PiperOrigin-RevId: 245235915 -- 2baa4df2e3284df925bfd728bab7d7bd60ae002e by Eric Fiselier : Remove need for `Windows.h` header in `waiter.h` Ideally we never want to drag in `windows.h` because it's non-modular and hijacks global identifiers like `ERROR` and `OPAQUE`. This patch changes our waiter implementation to store char buffers for `SRWLOCK` and `CONDITION_VARIABLE` instead of the types directly. PiperOrigin-RevId: 245189428 -- 33cfacd70c0d148d7590472dbcce38c93f2f7a34 by Matthew Brown : Internal change. PiperOrigin-RevId: 245092803 GitOrigin-RevId: 997d2a8d12d9395046b0bdfc2f206a0b2fe2f1f9 Change-Id: Icccd6cbe4b205096f6a71e114d135303ee4c1857 --- absl/hash/BUILD.bazel | 2 +- absl/hash/hash_test.cc | 8 ++-- absl/synchronization/internal/waiter.cc | 57 ++++++++++++++++++++++--- absl/synchronization/internal/waiter.h | 20 ++++++--- 4 files changed, 70 insertions(+), 17 deletions(-) diff --git a/absl/hash/BUILD.bazel b/absl/hash/BUILD.bazel index 8c2daf703..e1e6eae85 100644 --- a/absl/hash/BUILD.bazel +++ b/absl/hash/BUILD.bazel @@ -35,10 +35,10 @@ cc_library( copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ - ":city", "//absl/base:core_headers", "//absl/base:endian", "//absl/container:fixed_array", + "//absl/hash:city", "//absl/meta:type_traits", "//absl/numeric:int128", "//absl/strings", diff --git a/absl/hash/hash_test.cc b/absl/hash/hash_test.cc index af9593850..92c64ad50 100644 --- a/absl/hash/hash_test.cc +++ b/absl/hash/hash_test.cc @@ -425,10 +425,10 @@ TEST(HashValueTest, Maps) { } template -struct IsHashCallble : std::false_type {}; +struct IsHashCallable : std::false_type {}; template -struct IsHashCallble>()( +struct IsHashCallable>()( std::declval()))>> : std::true_type {}; template @@ -445,7 +445,7 @@ TEST(IsHashableTest, ValidHash) { EXPECT_TRUE(std::is_move_constructible>::value); EXPECT_TRUE(absl::is_copy_assignable>::value); EXPECT_TRUE(absl::is_move_assignable>::value); - EXPECT_TRUE(IsHashCallble::value); + EXPECT_TRUE(IsHashCallable::value); EXPECT_TRUE(IsAggregateInitializable>::value); } @@ -458,7 +458,7 @@ TEST(IsHashableTest, PoisonHash) { EXPECT_FALSE(std::is_move_constructible>::value); EXPECT_FALSE(absl::is_copy_assignable>::value); EXPECT_FALSE(absl::is_move_assignable>::value); - EXPECT_FALSE(IsHashCallble::value); + EXPECT_FALSE(IsHashCallable::value); EXPECT_FALSE(IsAggregateInitializable>::value); } #endif // ABSL_META_INTERNAL_STD_HASH_SFINAE_FRIENDLY_ diff --git a/absl/synchronization/internal/waiter.cc b/absl/synchronization/internal/waiter.cc index 74b096501..f17ea56fd 100644 --- a/absl/synchronization/internal/waiter.cc +++ b/absl/synchronization/internal/waiter.cc @@ -40,6 +40,8 @@ #include #include #include +#include +#include #include "absl/base/internal/raw_logging.h" #include "absl/base/internal/thread_identity.h" @@ -328,6 +330,43 @@ void Waiter::Poke() { #elif ABSL_WAITER_MODE == ABSL_WAITER_MODE_WIN32 +class Waiter::WinHelper { + public: + static SRWLOCK *GetLock(Waiter *w) { + return reinterpret_cast(&w->mu_storage_); + } + + static CONDITION_VARIABLE *GetCond(Waiter *w) { + return reinterpret_cast(&w->cv_storage_); + } + + static_assert(sizeof(SRWLOCK) == sizeof(Waiter::SRWLockStorage), + "SRWLockStorage does not have the same size as SRWLOCK"); + static_assert( + alignof(SRWLOCK) == alignof(Waiter::SRWLockStorage), + "SRWLockStorage does not have the same alignment as SRWLOCK"); + + static_assert(sizeof(CONDITION_VARIABLE) == + sizeof(Waiter::ConditionVariableStorage), + "ABSL_CONDITION_VARIABLE_STORAGE does not have the same size " + "as CONDITION_VARIABLE"); + static_assert(alignof(CONDITION_VARIABLE) == + alignof(Waiter::ConditionVariableStorage), + "ConditionVariableStorage does not have the same " + "alignment as CONDITION_VARIABLE"); + + // The SRWLOCK and CONDITION_VARIABLE types must be trivially constuctible + // and destructible because we never call their constructors or destructors. + static_assert(std::is_trivially_constructible::value, + "The SRWLOCK type must be trivially constructible"); + static_assert(std::is_trivially_constructible::value, + "The CONDITION_VARIABLE type must be trivially constructible"); + static_assert(std::is_trivially_destructible::value, + "The SRWLOCK type must be trivially destructible"); + static_assert(std::is_trivially_destructible::value, + "The CONDITION_VARIABLE type must be trivially destructible"); +}; + class LockHolder { public: explicit LockHolder(SRWLOCK* mu) : mu_(mu) { @@ -346,14 +385,19 @@ class LockHolder { }; void Waiter::Init() { - InitializeSRWLock(&mu_); - InitializeConditionVariable(&cv_); + auto *mu = ::new (static_cast(&mu_storage_)) SRWLOCK; + auto *cv = ::new (static_cast(&cv_storage_)) CONDITION_VARIABLE; + InitializeSRWLock(mu); + InitializeConditionVariable(cv); waiter_count_.store(0, std::memory_order_relaxed); wakeup_count_.store(0, std::memory_order_relaxed); } bool Waiter::Wait(KernelTimeout t) { - LockHolder h(&mu_); + SRWLOCK *mu = WinHelper::GetLock(this); + CONDITION_VARIABLE *cv = WinHelper::GetCond(this); + + LockHolder h(mu); waiter_count_.fetch_add(1, std::memory_order_relaxed); // Loop until we find a wakeup to consume or timeout. @@ -371,8 +415,7 @@ bool Waiter::Wait(KernelTimeout t) { } // No wakeups available, time to wait. - if (!SleepConditionVariableSRW( - &cv_, &mu_, t.InMillisecondsFromNow(), 0)) { + if (!SleepConditionVariableSRW(cv, mu, t.InMillisecondsFromNow(), 0)) { // GetLastError() returns a Win32 DWORD, but we assign to // unsigned long to simplify the ABSL_RAW_LOG case below. The uniform // initialization guarantees this is not a narrowing conversion. @@ -399,11 +442,11 @@ void Waiter::Poke() { return; } // Potentially a waker. Take the lock and check again. - LockHolder h(&mu_); + LockHolder h(WinHelper::GetLock(this)); if (waiter_count_.load(std::memory_order_relaxed) == 0) { return; } - WakeConditionVariable(&cv_); + WakeConditionVariable(WinHelper::GetCond(this)); } #else diff --git a/absl/synchronization/internal/waiter.h b/absl/synchronization/internal/waiter.h index 66b4bebfb..a3e3124e4 100644 --- a/absl/synchronization/internal/waiter.h +++ b/absl/synchronization/internal/waiter.h @@ -18,9 +18,7 @@ #include "absl/base/config.h" -#ifdef _WIN32 -#include -#else +#ifndef _WIN32 #include #endif @@ -123,8 +121,20 @@ class Waiter { // primivitives. We are using SRWLOCK and CONDITION_VARIABLE // because they don't require a destructor to release system // resources. - SRWLOCK mu_; - CONDITION_VARIABLE cv_; + // + // However, we can't include Windows.h in our headers, so we use aligned + // storage buffers to define the storage. + using SRWLockStorage = + typename std::aligned_storage::type; + using ConditionVariableStorage = + typename std::aligned_storage::type; + + // WinHelper - Used to define utilities for accessing the lock and + // condition variable storage once the types are complete. + class WinHelper; + + SRWLockStorage mu_storage_; + ConditionVariableStorage cv_storage_; std::atomic waiter_count_; std::atomic wakeup_count_;