about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAbseil Team <absl-team@google.com>2019-04-25T15·01-0700
committerMatt Calabrese <calabrese@x.team>2019-04-25T17·31-0400
commitcd86d0d20ab167c33b23d3875db68d1d4bad3a3b (patch)
tree13b92d2cd9f22326fa5cf9564ca056407569b79c
parent33841c5c963aa9c3f096ef8e6c1e71624b941940 (diff)
Export of internal Abseil changes.
--
997d2a8d12d9395046b0bdfc2f206a0b2fe2f1f9 by Abseil Team <absl-team@google.com>:

Typo fix: IsHashCallble -> IsHashCallable

PiperOrigin-RevId: 245235915

--
2baa4df2e3284df925bfd728bab7d7bd60ae002e by Eric Fiselier <ericwf@google.com>:

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 <matthewbr@google.com>:

Internal change.

PiperOrigin-RevId: 245092803
GitOrigin-RevId: 997d2a8d12d9395046b0bdfc2f206a0b2fe2f1f9
Change-Id: Icccd6cbe4b205096f6a71e114d135303ee4c1857
-rw-r--r--absl/hash/BUILD.bazel2
-rw-r--r--absl/hash/hash_test.cc8
-rw-r--r--absl/synchronization/internal/waiter.cc57
-rw-r--r--absl/synchronization/internal/waiter.h20
4 files changed, 70 insertions, 17 deletions
diff --git a/absl/hash/BUILD.bazel b/absl/hash/BUILD.bazel
index 8c2daf703bcd..e1e6eae8516d 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 af9593850a4b..92c64ad509d8 100644
--- a/absl/hash/hash_test.cc
+++ b/absl/hash/hash_test.cc
@@ -425,10 +425,10 @@ TEST(HashValueTest, Maps) {
 }
 
 template <typename T, typename = void>
-struct IsHashCallble : std::false_type {};
+struct IsHashCallable : std::false_type {};
 
 template <typename T>
-struct IsHashCallble<T, absl::void_t<decltype(std::declval<absl::Hash<T>>()(
+struct IsHashCallable<T, absl::void_t<decltype(std::declval<absl::Hash<T>>()(
                             std::declval<const T&>()))>> : std::true_type {};
 
 template <typename T, typename = void>
@@ -445,7 +445,7 @@ TEST(IsHashableTest, ValidHash) {
   EXPECT_TRUE(std::is_move_constructible<absl::Hash<int>>::value);
   EXPECT_TRUE(absl::is_copy_assignable<absl::Hash<int>>::value);
   EXPECT_TRUE(absl::is_move_assignable<absl::Hash<int>>::value);
-  EXPECT_TRUE(IsHashCallble<int>::value);
+  EXPECT_TRUE(IsHashCallable<int>::value);
   EXPECT_TRUE(IsAggregateInitializable<absl::Hash<int>>::value);
 }
 
@@ -458,7 +458,7 @@ TEST(IsHashableTest, PoisonHash) {
   EXPECT_FALSE(std::is_move_constructible<absl::Hash<X>>::value);
   EXPECT_FALSE(absl::is_copy_assignable<absl::Hash<X>>::value);
   EXPECT_FALSE(absl::is_move_assignable<absl::Hash<X>>::value);
-  EXPECT_FALSE(IsHashCallble<X>::value);
+  EXPECT_FALSE(IsHashCallable<X>::value);
   EXPECT_FALSE(IsAggregateInitializable<absl::Hash<X>>::value);
 }
 #endif  // ABSL_META_INTERNAL_STD_HASH_SFINAE_FRIENDLY_
diff --git a/absl/synchronization/internal/waiter.cc b/absl/synchronization/internal/waiter.cc
index 74b0965018c5..f17ea56fdda3 100644
--- a/absl/synchronization/internal/waiter.cc
+++ b/absl/synchronization/internal/waiter.cc
@@ -40,6 +40,8 @@
 #include <atomic>
 #include <cassert>
 #include <cstdint>
+#include <new>
+#include <type_traits>
 
 #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<SRWLOCK *>(&w->mu_storage_);
+  }
+
+  static CONDITION_VARIABLE *GetCond(Waiter *w) {
+    return reinterpret_cast<CONDITION_VARIABLE *>(&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<SRWLOCK>::value,
+                "The SRWLOCK type must be trivially constructible");
+  static_assert(std::is_trivially_constructible<CONDITION_VARIABLE>::value,
+                "The CONDITION_VARIABLE type must be trivially constructible");
+  static_assert(std::is_trivially_destructible<SRWLOCK>::value,
+                "The SRWLOCK type must be trivially destructible");
+  static_assert(std::is_trivially_destructible<CONDITION_VARIABLE>::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<void *>(&mu_storage_)) SRWLOCK;
+  auto *cv = ::new (static_cast<void *>(&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 66b4bebfb532..a3e3124e4417 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 <windows.h>
-#else
+#ifndef _WIN32
 #include <pthread.h>
 #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<sizeof(void*), alignof(void*)>::type;
+  using ConditionVariableStorage =
+      typename std::aligned_storage<sizeof(void*), alignof(void*)>::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<int> waiter_count_;
   std::atomic<int> wakeup_count_;