diff options
Diffstat (limited to 'absl/synchronization')
29 files changed, 614 insertions, 337 deletions
diff --git a/absl/synchronization/BUILD.bazel b/absl/synchronization/BUILD.bazel index 372874e1..e63b1d16 100644 --- a/absl/synchronization/BUILD.bazel +++ b/absl/synchronization/BUILD.bazel @@ -88,6 +88,9 @@ cc_test( size = "small", srcs = ["barrier_test.cc"], copts = ABSL_TEST_COPTS, + tags = [ + "no_test_wasm", + ], deps = [ ":synchronization", "//absl/time", @@ -100,6 +103,9 @@ cc_test( size = "small", srcs = ["blocking_counter_test.cc"], copts = ABSL_TEST_COPTS, + tags = [ + "no_test_wasm", + ], deps = [ ":synchronization", "//absl/time", @@ -138,6 +144,9 @@ cc_library( name = "thread_pool", testonly = 1, hdrs = ["internal/thread_pool.h"], + visibility = [ + "//absl:__subpackages__", + ], deps = [ ":synchronization", "//absl/base:core_headers", @@ -149,6 +158,7 @@ cc_test( size = "large", srcs = ["mutex_test.cc"], copts = ABSL_TEST_COPTS, + shard_count = 25, deps = [ ":synchronization", ":thread_pool", @@ -160,18 +170,32 @@ cc_test( ], ) -cc_test( - name = "mutex_benchmark", +cc_library( + name = "mutex_benchmark_common", + testonly = 1, srcs = ["mutex_benchmark.cc"], - copts = ABSL_TEST_COPTS, - tags = ["benchmark"], - visibility = ["//visibility:private"], + copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl/synchronization:__pkg__", + ], deps = [ ":synchronization", ":thread_pool", "//absl/base", + "//absl/base:base_internal", "@com_github_google_benchmark//:benchmark_main", ], + alwayslink = 1, +) + +cc_binary( + name = "mutex_benchmark", + testonly = 1, + copts = ABSL_DEFAULT_COPTS, + visibility = ["//visibility:private"], + deps = [ + ":mutex_benchmark_common", + ], ) cc_test( @@ -205,6 +229,7 @@ cc_test( name = "per_thread_sem_test", size = "medium", copts = ABSL_TEST_COPTS, + tags = ["no_test_wasm"], deps = [ ":per_thread_sem_test_common", ":synchronization", @@ -225,6 +250,7 @@ cc_test( "//absl:windows": [], "//conditions:default": ["-pthread"], }), + tags = ["no_test_ios_x86_64"], deps = [ ":synchronization", "//absl/base", diff --git a/absl/synchronization/CMakeLists.txt b/absl/synchronization/CMakeLists.txt index c19f5725..de0d7b7d 100644 --- a/absl/synchronization/CMakeLists.txt +++ b/absl/synchronization/CMakeLists.txt @@ -34,7 +34,7 @@ list(APPEND SYNCHRONIZATION_INTERNAL_HEADERS # synchronization library -list(APPEND SYNCHRONIZATION_SRC +list(APPEND SYNCHRONIZATION_SRC "barrier.cc" "blocking_counter.cc" "internal/create_thread_identity.cc" diff --git a/absl/synchronization/barrier.cc b/absl/synchronization/barrier.cc index 545bb891..ee66c240 100644 --- a/absl/synchronization/barrier.cc +++ b/absl/synchronization/barrier.cc @@ -18,7 +18,7 @@ #include "absl/synchronization/mutex.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { // Return whether int *arg is zero. static bool IsZero(void *arg) { @@ -48,5 +48,5 @@ bool Barrier::Block() { return this->num_to_exit_ == 0; } -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/barrier.h b/absl/synchronization/barrier.h index ccae0a4c..77ac3602 100644 --- a/absl/synchronization/barrier.h +++ b/absl/synchronization/barrier.h @@ -23,7 +23,7 @@ #include "absl/synchronization/mutex.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { // Barrier // @@ -74,6 +74,6 @@ class Barrier { int num_to_exit_ GUARDED_BY(lock_); }; -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_BARRIER_H_ diff --git a/absl/synchronization/blocking_counter.cc b/absl/synchronization/blocking_counter.cc index f998099a..82d889a9 100644 --- a/absl/synchronization/blocking_counter.cc +++ b/absl/synchronization/blocking_counter.cc @@ -17,7 +17,7 @@ #include "absl/base/internal/raw_logging.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { // Return whether int *arg is zero. static bool IsZero(void *arg) { @@ -53,5 +53,5 @@ void BlockingCounter::Wait() { // after we return from this method. } -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/blocking_counter.h b/absl/synchronization/blocking_counter.h index 08b6f58c..554e396c 100644 --- a/absl/synchronization/blocking_counter.h +++ b/absl/synchronization/blocking_counter.h @@ -24,7 +24,7 @@ #include "absl/synchronization/mutex.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { // BlockingCounter // @@ -93,7 +93,7 @@ class BlockingCounter { int num_waiting_ GUARDED_BY(lock_); }; -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_BLOCKING_COUNTER_H_ diff --git a/absl/synchronization/blocking_counter_test.cc b/absl/synchronization/blocking_counter_test.cc index 486aa9b1..b3b55dd7 100644 --- a/absl/synchronization/blocking_counter_test.cc +++ b/absl/synchronization/blocking_counter_test.cc @@ -22,7 +22,7 @@ #include "absl/time/time.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace { void PauseAndDecreaseCounter(BlockingCounter* counter, int* done) { @@ -64,5 +64,5 @@ TEST(BlockingCounterTest, BasicFunctionality) { } } // namespace -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/internal/create_thread_identity.cc b/absl/synchronization/internal/create_thread_identity.cc index 38cc7e2f..f27f16da 100644 --- a/absl/synchronization/internal/create_thread_identity.cc +++ b/absl/synchronization/internal/create_thread_identity.cc @@ -27,7 +27,7 @@ #include "absl/synchronization/internal/per_thread_sem.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { // ThreadIdentity storage is persistent, we maintain a free-list of previously @@ -108,7 +108,7 @@ base_internal::ThreadIdentity* CreateThreadIdentity() { } } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_LOW_LEVEL_ALLOC_MISSING diff --git a/absl/synchronization/internal/create_thread_identity.h b/absl/synchronization/internal/create_thread_identity.h index 05b5f7e1..1132d516 100644 --- a/absl/synchronization/internal/create_thread_identity.h +++ b/absl/synchronization/internal/create_thread_identity.h @@ -29,7 +29,7 @@ #include "absl/base/port.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { // Allocates and attaches a ThreadIdentity object for the calling thread. @@ -50,6 +50,6 @@ inline base_internal::ThreadIdentity* GetOrCreateCurrentThreadIdentity() { } } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_INTERNAL_CREATE_THREAD_IDENTITY_H_ diff --git a/absl/synchronization/internal/graphcycles.cc b/absl/synchronization/internal/graphcycles.cc index 5a015844..139be0f5 100644 --- a/absl/synchronization/internal/graphcycles.cc +++ b/absl/synchronization/internal/graphcycles.cc @@ -44,7 +44,7 @@ // Do not use STL. This module does not use standard memory allocation. namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { namespace { @@ -205,8 +205,7 @@ class NodeSet { } private: - static const int32_t kEmpty; - static const int32_t kDel; + enum : int32_t { kEmpty = -1, kDel = -2 }; Vec<int32_t> table_; uint32_t occupied_; // Count of non-empty slots (includes deleted slots) @@ -256,9 +255,6 @@ class NodeSet { NodeSet& operator=(const NodeSet&) = delete; }; -const int32_t NodeSet::kEmpty = -1; -const int32_t NodeSet::kDel = -2; - // We encode a node index and a node version in GraphId. The version // number is incremented when the GraphId is freed which automatically // invalidates all copies of the GraphId. @@ -695,7 +691,7 @@ int GraphCycles::GetStackTrace(GraphId id, void*** ptr) { } } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_LOW_LEVEL_ALLOC_MISSING diff --git a/absl/synchronization/internal/graphcycles.h b/absl/synchronization/internal/graphcycles.h index e43ae26b..6609ea06 100644 --- a/absl/synchronization/internal/graphcycles.h +++ b/absl/synchronization/internal/graphcycles.h @@ -41,7 +41,7 @@ #include <cstdint> namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { // Opaque identifier for a graph node. @@ -133,7 +133,7 @@ class GraphCycles { }; } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif diff --git a/absl/synchronization/internal/graphcycles_test.cc b/absl/synchronization/internal/graphcycles_test.cc index 09332bad..4dc2bdc5 100644 --- a/absl/synchronization/internal/graphcycles_test.cc +++ b/absl/synchronization/internal/graphcycles_test.cc @@ -25,7 +25,7 @@ #include "absl/base/macros.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { // We emulate a GraphCycles object with a node vector and an edge vector. @@ -460,5 +460,5 @@ TEST_F(GraphCyclesTest, ManyEdges) { } } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/internal/kernel_timeout.h b/absl/synchronization/internal/kernel_timeout.h index 3d3dc0cb..34ae94ec 100644 --- a/absl/synchronization/internal/kernel_timeout.h +++ b/absl/synchronization/internal/kernel_timeout.h @@ -25,9 +25,6 @@ #ifndef ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ #define ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ -#ifdef _WIN32 -#include <intsafe.h> -#endif #include <time.h> #include <algorithm> #include <limits> @@ -37,7 +34,7 @@ #include "absl/time/time.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { class Futex; @@ -80,7 +77,7 @@ class KernelTimeout { if (x <= 0) x = 1; // A time larger than what can be represented to the kernel is treated // as no timeout. - if (x == std::numeric_limits<int64_t>::max()) x = 0; + if (x == (std::numeric_limits<int64_t>::max)()) x = 0; return x; } @@ -94,7 +91,7 @@ class KernelTimeout { ERROR, "Tried to create a timespec from a non-timeout; never do this."); // But we'll try to continue sanely. no-timeout ~= saturated timeout. - n = std::numeric_limits<int64_t>::max(); + n = (std::numeric_limits<int64_t>::max)(); } // Kernel APIs validate timespecs as being at or after the epoch, @@ -105,7 +102,7 @@ class KernelTimeout { struct timespec abstime; int64_t seconds = std::min(n / kNanosPerSecond, - int64_t{std::numeric_limits<time_t>::max()}); + int64_t{(std::numeric_limits<time_t>::max)()}); abstime.tv_sec = static_cast<time_t>(seconds); abstime.tv_nsec = static_cast<decltype(abstime.tv_nsec)>(n % kNanosPerSecond); @@ -118,9 +115,14 @@ class KernelTimeout { // Windows. Callers should recognize that the return value is a // relative duration (it should be recomputed by calling this method // in the case of a spurious wakeup). - DWORD InMillisecondsFromNow() const { + // This header file may be included transitively by public header files, + // so we define our own DWORD and INFINITE instead of getting them from + // <intsafe.h> and <WinBase.h>. + typedef unsigned long DWord; // NOLINT + DWord InMillisecondsFromNow() const { + constexpr DWord kInfinite = (std::numeric_limits<DWord>::max)(); if (!has_timeout()) { - return INFINITE; + return kInfinite; } // The use of absl::Now() to convert from absolute time to // relative time means that absl::Now() cannot use anything that @@ -129,13 +131,13 @@ class KernelTimeout { if (ns_ >= now) { // Round up so that Now() + ms_from_now >= ns_. constexpr uint64_t max_nanos = - std::numeric_limits<int64_t>::max() - 999999u; + (std::numeric_limits<int64_t>::max)() - 999999u; uint64_t ms_from_now = (std::min<uint64_t>(max_nanos, ns_ - now) + 999999u) / 1000000u; - if (ms_from_now > std::numeric_limits<DWORD>::max()) { - return INFINITE; + if (ms_from_now > kInfinite) { + return kInfinite; } - return static_cast<DWORD>(ms_from_now); + return static_cast<DWord>(ms_from_now); } return 0; } @@ -146,6 +148,6 @@ class KernelTimeout { }; } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ diff --git a/absl/synchronization/internal/mutex_nonprod.cc b/absl/synchronization/internal/mutex_nonprod.cc index a8071a96..4b0b8bcd 100644 --- a/absl/synchronization/internal/mutex_nonprod.cc +++ b/absl/synchronization/internal/mutex_nonprod.cc @@ -31,7 +31,7 @@ #include "absl/time/time.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { namespace { @@ -316,5 +316,5 @@ bool Condition::Eval() const { void RegisterSymbolizer(bool (*)(const void*, char*, int)) {} -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/internal/mutex_nonprod.inc b/absl/synchronization/internal/mutex_nonprod.inc index 2d06285f..0ae4c0ea 100644 --- a/absl/synchronization/internal/mutex_nonprod.inc +++ b/absl/synchronization/internal/mutex_nonprod.inc @@ -36,7 +36,7 @@ #endif namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { class Condition; namespace synchronization_internal { @@ -254,5 +254,5 @@ class SynchronizationStorage { }; } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/internal/per_thread_sem.cc b/absl/synchronization/internal/per_thread_sem.cc index 53b789d6..9de2d136 100644 --- a/absl/synchronization/internal/per_thread_sem.cc +++ b/absl/synchronization/internal/per_thread_sem.cc @@ -25,7 +25,7 @@ #include "absl/synchronization/internal/waiter.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { void PerThreadSem::SetThreadBlockedCounter(std::atomic<int> *counter) { @@ -59,7 +59,7 @@ void PerThreadSem::Tick(base_internal::ThreadIdentity *identity) { } } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl extern "C" { diff --git a/absl/synchronization/internal/per_thread_sem.h b/absl/synchronization/internal/per_thread_sem.h index fc64f768..6efd5951 100644 --- a/absl/synchronization/internal/per_thread_sem.h +++ b/absl/synchronization/internal/per_thread_sem.h @@ -32,7 +32,7 @@ #include "absl/synchronization/internal/kernel_timeout.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { class Mutex; @@ -81,7 +81,7 @@ class PerThreadSem { }; } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl // In some build configurations we pass --detect-odr-violations to the diff --git a/absl/synchronization/internal/per_thread_sem_test.cc b/absl/synchronization/internal/per_thread_sem_test.cc index 63c8e56a..18b2458b 100644 --- a/absl/synchronization/internal/per_thread_sem_test.cc +++ b/absl/synchronization/internal/per_thread_sem_test.cc @@ -33,7 +33,7 @@ // primitives which might use PerThreadSem, most notably absl::Mutex. namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { class SimpleSemaphore { @@ -154,12 +154,15 @@ TEST_F(PerThreadSemTest, WithTimeout) { TEST_F(PerThreadSemTest, Timeouts) { absl::Time timeout = absl::Now() + absl::Milliseconds(50); + // Allow for a slight early return, to account for quality of implementation + // issues on various platforms. + const absl::Duration slop = absl::Microseconds(200); EXPECT_FALSE(Wait(timeout)); - EXPECT_LE(timeout, absl::Now()); + EXPECT_LE(timeout, absl::Now() + slop); absl::Time negative_timeout = absl::UnixEpoch() - absl::Milliseconds(100); EXPECT_FALSE(Wait(negative_timeout)); - EXPECT_LE(negative_timeout, absl::Now()); // trivially true :) + EXPECT_LE(negative_timeout, absl::Now() + slop); // trivially true :) Post(GetOrCreateCurrentThreadIdentity()); // The wait here has an expired timeout, but we have a wake to consume, @@ -170,5 +173,5 @@ TEST_F(PerThreadSemTest, Timeouts) { } // namespace } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/internal/thread_pool.h b/absl/synchronization/internal/thread_pool.h index 82dedbf5..66c7546b 100644 --- a/absl/synchronization/internal/thread_pool.h +++ b/absl/synchronization/internal/thread_pool.h @@ -25,7 +25,7 @@ #include "absl/synchronization/mutex.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { // A simple ThreadPool implementation for tests. @@ -86,7 +86,7 @@ class ThreadPool { }; } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_INTERNAL_THREAD_POOL_H_ diff --git a/absl/synchronization/internal/waiter.cc b/absl/synchronization/internal/waiter.cc index ad86acce..76fdd861 100644 --- a/absl/synchronization/internal/waiter.cc +++ b/absl/synchronization/internal/waiter.cc @@ -46,7 +46,7 @@ #include "absl/synchronization/internal/kernel_timeout.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { static void MaybeBecomeIdle() { @@ -410,5 +410,5 @@ void Waiter::Poke() { #endif } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/internal/waiter.h b/absl/synchronization/internal/waiter.h index 1c284c0a..2b737260 100644 --- a/absl/synchronization/internal/waiter.h +++ b/absl/synchronization/internal/waiter.h @@ -53,7 +53,7 @@ #endif namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace synchronization_internal { // Waiter is an OS-specific semaphore. @@ -135,7 +135,7 @@ class Waiter { }; } // namespace synchronization_internal -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_INTERNAL_WAITER_H_ diff --git a/absl/synchronization/lifetime_test.cc b/absl/synchronization/lifetime_test.cc index 90c9009b..b7360c29 100644 --- a/absl/synchronization/lifetime_test.cc +++ b/absl/synchronization/lifetime_test.cc @@ -72,23 +72,19 @@ void ThreadTwo(absl::Mutex* mutex, absl::CondVar* condvar, // Launch thread 1 and thread 2, and block on their completion. // If any of 'mutex', 'condvar', or 'notification' is nullptr, use a locally // constructed instance instead. -void RunTests(absl::Mutex* mutex, absl::CondVar* condvar, - absl::Notification* notification) { +void RunTests(absl::Mutex* mutex, absl::CondVar* condvar) { absl::Mutex default_mutex; absl::CondVar default_condvar; - absl::Notification default_notification; + absl::Notification notification; if (!mutex) { mutex = &default_mutex; } if (!condvar) { condvar = &default_condvar; } - if (!notification) { - notification = &default_notification; - } bool state = false; - std::thread thread_one(ThreadOne, mutex, condvar, notification, &state); - std::thread thread_two(ThreadTwo, mutex, condvar, notification, &state); + std::thread thread_one(ThreadOne, mutex, condvar, ¬ification, &state); + std::thread thread_two(ThreadTwo, mutex, condvar, ¬ification, &state); thread_one.join(); thread_two.join(); } @@ -96,8 +92,7 @@ void RunTests(absl::Mutex* mutex, absl::CondVar* condvar, void TestLocals() { absl::Mutex mutex; absl::CondVar condvar; - absl::Notification notification; - RunTests(&mutex, &condvar, ¬ification); + RunTests(&mutex, &condvar); } // Global variables during start and termination diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 14e24a76..9f8d6cd7 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -71,7 +71,7 @@ ABSL_ATTRIBUTE_WEAK void AbslInternalMutexYield() { std::this_thread::yield(); } } // extern "C" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { namespace { @@ -299,7 +299,7 @@ static struct SynchEvent { // this is a trivial hash table for the events // set "bits" in the word there (waiting until lockbit is clear before doing // so), and return a refcounted reference that will remain valid until // UnrefSynchEvent() is called. If a new SynchEvent is allocated, -// the std::string name is copied into it. +// the string name is copied into it. // When used with a mutex, the caller should also ensure that kMuEvent // is set in the mutex word, and similarly for condition variables and kCVEvent. static SynchEvent *EnsureSynchEvent(std::atomic<intptr_t> *addr, @@ -1828,8 +1828,8 @@ bool Mutex::LockSlowWithDeadline(MuHow how, const Condition *cond, cond == nullptr || EvalConditionAnnotated(cond, this, true, how); } -// RAW_CHECK_FMT() takes a condition, a printf-style format std::string, and -// the printf-style argument list. The format std::string must be a literal. +// RAW_CHECK_FMT() takes a condition, a printf-style format string, and +// the printf-style argument list. The format string must be a literal. // Arguments after the first are not evaluated unless the condition is true. #define RAW_CHECK_FMT(cond, ...) \ do { \ @@ -1976,7 +1976,7 @@ void Mutex::LockSlowLoop(SynchWaitParams *waitp, int flags) { // Unlock this mutex, which is held by the current thread. // If waitp is non-zero, it must be the wait parameters for the current thread // which holds the lock but is not runnable because its condition is false -// or it n the process of blocking on a condition variable; it must requeue +// or it is in the process of blocking on a condition variable; it must requeue // itself on the mutex/condvar to wait for its condition to become true. void Mutex::UnlockSlow(SynchWaitParams *waitp) { intptr_t v = mu_.load(std::memory_order_relaxed); @@ -2685,5 +2685,5 @@ bool Condition::GuaranteedEqual(const Condition *a, const Condition *b) { a->arg_ == b->arg_ && a->method_ == b->method_; } -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/mutex.h b/absl/synchronization/mutex.h index b5276037..ce97707f 100644 --- a/absl/synchronization/mutex.h +++ b/absl/synchronization/mutex.h @@ -24,7 +24,7 @@ // Unlike a `std::mutex`, the Abseil `Mutex` provides the following additional // features: // * Conditional predicates intrinsic to the `Mutex` object -// * Reader/writer locks, in addition to standard exclusive/writer locks +// * Shared/reader locks, in addition to standard exclusive/writer locks // * Deadlock detection and debug support. // // The following helper classes are also defined within this file: @@ -81,7 +81,7 @@ #endif namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { class Condition; struct SynchWaitParams; @@ -291,7 +291,7 @@ class LOCKABLE Mutex { // Mutex::ReaderLockWhen() // Mutex::WriterLockWhen() // - // Blocks until simultaneously both `cond` is `true` and this` Mutex` can + // Blocks until simultaneously both `cond` is `true` and this `Mutex` can // be acquired, then atomically acquires this `Mutex`. `LockWhen()` is // logically equivalent to `*Lock(); Await();` though they may have different // performance characteristics. @@ -559,7 +559,7 @@ class SCOPED_LOCKABLE ReaderMutexLock { // WriterMutexLock // // The `WriterMutexLock` is a helper class, like `MutexLock`, which acquires and -// releases a write (exclusive) lock on a `Mutex` va RAII. +// releases a write (exclusive) lock on a `Mutex` via RAII. class SCOPED_LOCKABLE WriterMutexLock { public: explicit WriterMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu) @@ -585,7 +585,7 @@ class SCOPED_LOCKABLE WriterMutexLock { // ----------------------------------------------------------------------------- // // As noted above, `Mutex` contains a number of member functions which take a -// `Condition` as a argument; clients can wait for conditions to become `true` +// `Condition` as an argument; clients can wait for conditions to become `true` // before attempting to acquire the mutex. These sections are known as // "condition critical" sections. To use a `Condition`, you simply need to // construct it, and use within an appropriate `Mutex` member function; @@ -963,7 +963,7 @@ void RegisterMutexTracer(void (*fn)(const char *msg, const void *obj, // // The function pointer registered here will be called here on various CondVar // events. The callback is given an opaque handle to the CondVar object and -// a std::string identifying the event. This is thread-safe, but only a single +// a string identifying the event. This is thread-safe, but only a single // tracer can be registered. // // Events that can be sent are "Wait", "Unwait", "Signal wakeup", and @@ -1015,7 +1015,7 @@ enum class OnDeadlockCycle { // the manner chosen here. void SetMutexDeadlockDetectionMode(OnDeadlockCycle mode); -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl // In some build configurations we pass --detect-odr-violations to the diff --git a/absl/synchronization/mutex_benchmark.cc b/absl/synchronization/mutex_benchmark.cc index 30a52355..2652bb97 100644 --- a/absl/synchronization/mutex_benchmark.cc +++ b/absl/synchronization/mutex_benchmark.cc @@ -12,16 +12,154 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include <cstdint> +#include <mutex> // NOLINT(build/c++11) #include <vector> -#include "benchmark/benchmark.h" -#include "absl/base/internal/sysinfo.h" +#include "absl/base/internal/cycleclock.h" +#include "absl/base/internal/spinlock.h" #include "absl/synchronization/blocking_counter.h" #include "absl/synchronization/internal/thread_pool.h" #include "absl/synchronization/mutex.h" +#include "benchmark/benchmark.h" namespace { +void BM_Mutex(benchmark::State& state) { + static absl::Mutex* mu = new absl::Mutex; + for (auto _ : state) { + absl::MutexLock lock(mu); + } +} +BENCHMARK(BM_Mutex)->UseRealTime()->Threads(1)->ThreadPerCpu(); + +static void DelayNs(int64_t ns, int* data) { + int64_t end = absl::base_internal::CycleClock::Now() + + ns * absl::base_internal::CycleClock::Frequency() / 1e9; + while (absl::base_internal::CycleClock::Now() < end) { + ++(*data); + benchmark::DoNotOptimize(*data); + } +} + +template <typename MutexType> +class RaiiLocker { + public: + explicit RaiiLocker(MutexType* mu) : mu_(mu) { mu_->Lock(); } + ~RaiiLocker() { mu_->Unlock(); } + private: + MutexType* mu_; +}; + +template <> +class RaiiLocker<std::mutex> { + public: + explicit RaiiLocker(std::mutex* mu) : mu_(mu) { mu_->lock(); } + ~RaiiLocker() { mu_->unlock(); } + private: + std::mutex* mu_; +}; + +template <typename MutexType> +void BM_Contended(benchmark::State& state) { + struct Shared { + MutexType mu; + int data = 0; + }; + static auto* shared = new Shared; + int local = 0; + for (auto _ : state) { + // Here we model both local work outside of the critical section as well as + // some work inside of the critical section. The idea is to capture some + // more or less realisitic contention levels. + // If contention is too low, the benchmark won't measure anything useful. + // If contention is unrealistically high, the benchmark will favor + // bad mutex implementations that block and otherwise distract threads + // from the mutex and shared state for as much as possible. + // To achieve this amount of local work is multiplied by number of threads + // to keep ratio between local work and critical section approximately + // equal regardless of number of threads. + DelayNs(100 * state.threads, &local); + RaiiLocker<MutexType> locker(&shared->mu); + DelayNs(state.range(0), &shared->data); + } +} + +BENCHMARK_TEMPLATE(BM_Contended, absl::Mutex) + ->UseRealTime() + // ThreadPerCpu poorly handles non-power-of-two CPU counts. + ->Threads(1) + ->Threads(2) + ->Threads(4) + ->Threads(6) + ->Threads(8) + ->Threads(12) + ->Threads(16) + ->Threads(24) + ->Threads(32) + ->Threads(48) + ->Threads(64) + ->Threads(96) + ->Threads(128) + ->Threads(192) + ->Threads(256) + // Some empirically chosen amounts of work in critical section. + // 1 is low contention, 200 is high contention and few values in between. + ->Arg(1) + ->Arg(20) + ->Arg(50) + ->Arg(200); + +BENCHMARK_TEMPLATE(BM_Contended, absl::base_internal::SpinLock) + ->UseRealTime() + // ThreadPerCpu poorly handles non-power-of-two CPU counts. + ->Threads(1) + ->Threads(2) + ->Threads(4) + ->Threads(6) + ->Threads(8) + ->Threads(12) + ->Threads(16) + ->Threads(24) + ->Threads(32) + ->Threads(48) + ->Threads(64) + ->Threads(96) + ->Threads(128) + ->Threads(192) + ->Threads(256) + // Some empirically chosen amounts of work in critical section. + // 1 is low contention, 200 is high contention and few values in between. + ->Arg(1) + ->Arg(20) + ->Arg(50) + ->Arg(200); + +BENCHMARK_TEMPLATE(BM_Contended, std::mutex) + ->UseRealTime() + // ThreadPerCpu poorly handles non-power-of-two CPU counts. + ->Threads(1) + ->Threads(2) + ->Threads(4) + ->Threads(6) + ->Threads(8) + ->Threads(12) + ->Threads(16) + ->Threads(24) + ->Threads(32) + ->Threads(48) + ->Threads(64) + ->Threads(96) + ->Threads(128) + ->Threads(192) + ->Threads(256) + // Some empirically chosen amounts of work in critical section. + // 1 is low contention, 200 is high contention and few values in between. + ->Arg(1) + ->Arg(20) + ->Arg(50) + ->Arg(200); + // Measure the overhead of conditions on mutex release (when they must be // evaluated). Mutex has (some) support for equivalence classes allowing // Conditions with the same function/argument to potentially not be multiply @@ -74,21 +212,12 @@ void BM_ConditionWaiters(benchmark::State& state) { mu.Unlock(); } -#ifdef THREAD_SANITIZER -// ThreadSanitizer can't handle 8192 threads. -constexpr int kMaxConditionWaiters = 2048; -#else +// Some configurations have higher thread limits than others. +#if defined(__linux__) && !defined(THREAD_SANITIZER) constexpr int kMaxConditionWaiters = 8192; +#else +constexpr int kMaxConditionWaiters = 1024; #endif BENCHMARK(BM_ConditionWaiters)->RangePair(0, 2, 1, kMaxConditionWaiters); -void BM_ContendedMutex(benchmark::State& state) { - static absl::Mutex* mu = new absl::Mutex; - for (auto _ : state) { - absl::MutexLock lock(mu); - } -} -BENCHMARK(BM_ContendedMutex)->Threads(1); -BENCHMARK(BM_ContendedMutex)->ThreadPerCpu(); - } // namespace diff --git a/absl/synchronization/mutex_test.cc b/absl/synchronization/mutex_test.cc index 53b93784..b2820e20 100644 --- a/absl/synchronization/mutex_test.cc +++ b/absl/synchronization/mutex_test.cc @@ -29,6 +29,7 @@ #include <vector> #include "gtest/gtest.h" +#include "absl/base/attributes.h" #include "absl/base/internal/raw_logging.h" #include "absl/base/internal/sysinfo.h" #include "absl/memory/memory.h" @@ -54,8 +55,8 @@ CreateDefaultPool() { // Hack to schedule a function to run on a thread pool thread after a // duration has elapsed. static void ScheduleAfter(absl::synchronization_internal::ThreadPool *tp, - const std::function<void()> &func, - absl::Duration after) { + absl::Duration after, + const std::function<void()> &func) { tp->Schedule([func, after] { absl::SleepFor(after); func(); @@ -1150,249 +1151,369 @@ TEST(Mutex, DeadlockIdBug) NO_THREAD_SAFETY_ANALYSIS { // and so never expires/passes, and one that will expire/pass in the near // future. -// Encapsulate a Mutex-protected bool with its associated Condition/CondVar. -class Cond { - public: - explicit Cond(bool use_deadline) : use_deadline_(use_deadline), c_(&b_) {} - - void Set(bool v) { - absl::MutexLock lock(&mu_); - b_ = v; +static absl::Duration TimeoutTestAllowedSchedulingDelay() { + // Note: we use a function here because Microsoft Visual Studio fails to + // properly initialize constexpr static absl::Duration variables. + return absl::Milliseconds(150); +} + +// Returns true if `actual_delay` is close enough to `expected_delay` to pass +// the timeouts/deadlines test. Otherwise, logs warnings and returns false. +ABSL_MUST_USE_RESULT +static bool DelayIsWithinBounds(absl::Duration expected_delay, + absl::Duration actual_delay) { + bool pass = true; + // Do not allow the observed delay to be less than expected. This may occur + // in practice due to clock skew or when the synchronization primitives use a + // different clock than absl::Now(), but these cases should be handled by the + // the retry mechanism in each TimeoutTest. + if (actual_delay < expected_delay) { + ABSL_RAW_LOG(WARNING, + "Actual delay %s was too short, expected %s (difference %s)", + absl::FormatDuration(actual_delay).c_str(), + absl::FormatDuration(expected_delay).c_str(), + absl::FormatDuration(actual_delay - expected_delay).c_str()); + pass = false; } - - bool AwaitWithTimeout(absl::Duration timeout) { - absl::MutexLock lock(&mu_); - return use_deadline_ ? mu_.AwaitWithDeadline(c_, absl::Now() + timeout) - : mu_.AwaitWithTimeout(c_, timeout); + // If the expected delay is <= zero then allow a small error tolerance, since + // we do not expect context switches to occur during test execution. + // Otherwise, thread scheduling delays may be substantial in rare cases, so + // tolerate up to kTimeoutTestAllowedSchedulingDelay of error. + absl::Duration tolerance = expected_delay <= absl::ZeroDuration() + ? absl::Milliseconds(10) + : TimeoutTestAllowedSchedulingDelay(); + if (actual_delay > expected_delay + tolerance) { + ABSL_RAW_LOG(WARNING, + "Actual delay %s was too long, expected %s (difference %s)", + absl::FormatDuration(actual_delay).c_str(), + absl::FormatDuration(expected_delay).c_str(), + absl::FormatDuration(actual_delay - expected_delay).c_str()); + pass = false; } + return pass; +} + +// Parameters for TimeoutTest, below. +struct TimeoutTestParam { + // The file and line number (used for logging purposes only). + const char *from_file; + int from_line; + + // Should the absolute deadline API based on absl::Time be tested? If false, + // the relative deadline API based on absl::Duration is tested. + bool use_absolute_deadline; + + // The deadline/timeout used when calling the API being tested + // (e.g. Mutex::LockWhenWithDeadline). + absl::Duration wait_timeout; + + // The delay before the condition will be set true by the test code. If zero + // or negative, the condition is set true immediately (before calling the API + // being tested). Otherwise, if infinite, the condition is never set true. + // Otherwise a closure is scheduled for the future that sets the condition + // true. + absl::Duration satisfy_condition_delay; + + // The expected result of the condition after the call to the API being + // tested. Generally `true` means the condition was true when the API returns, + // `false` indicates an expected timeout. + bool expected_result; + + // The expected delay before the API under test returns. This is inherently + // flaky, so some slop is allowed (see `DelayIsWithinBounds` above), and the + // test keeps trying indefinitely until this constraint passes. + absl::Duration expected_delay; +}; - bool LockWhenWithTimeout(absl::Duration timeout) { - bool b = use_deadline_ ? mu_.LockWhenWithDeadline(c_, absl::Now() + timeout) - : mu_.LockWhenWithTimeout(c_, timeout); - mu_.Unlock(); - return b; +// Print a `TimeoutTestParam` to a debug log. +std::ostream &operator<<(std::ostream &os, const TimeoutTestParam ¶m) { + return os << "from: " << param.from_file << ":" << param.from_line + << " use_absolute_deadline: " + << (param.use_absolute_deadline ? "true" : "false") + << " wait_timeout: " << param.wait_timeout + << " satisfy_condition_delay: " << param.satisfy_condition_delay + << " expected_result: " + << (param.expected_result ? "true" : "false") + << " expected_delay: " << param.expected_delay; +} + +std::string FormatString(const TimeoutTestParam ¶m) { + std::ostringstream os; + os << param; + return os.str(); +} + +// Like `thread::Executor::ScheduleAt` except: +// a) Delays zero or negative are executed immediately in the current thread. +// b) Infinite delays are never scheduled. +// c) Calls this test's `ScheduleAt` helper instead of using `pool` directly. +static void RunAfterDelay(absl::Duration delay, + absl::synchronization_internal::ThreadPool *pool, + const std::function<void()> &callback) { + if (delay <= absl::ZeroDuration()) { + callback(); // immediate + } else if (delay != absl::InfiniteDuration()) { + ScheduleAfter(pool, delay, callback); } +} - bool ReaderLockWhenWithTimeout(absl::Duration timeout) { - bool b = use_deadline_ - ? mu_.ReaderLockWhenWithDeadline(c_, absl::Now() + timeout) - : mu_.ReaderLockWhenWithTimeout(c_, timeout); - mu_.ReaderUnlock(); - return b; - } +class TimeoutTest : public ::testing::Test, + public ::testing::WithParamInterface<TimeoutTestParam> {}; - void Await() { - absl::MutexLock lock(&mu_); - mu_.Await(c_); - } +std::vector<TimeoutTestParam> MakeTimeoutTestParamValues() { + // The `finite` delay is a finite, relatively short, delay. We make it larger + // than our allowed scheduling delay (slop factor) to avoid confusion when + // diagnosing test failures. The other constants here have clear meanings. + const absl::Duration finite = 3 * TimeoutTestAllowedSchedulingDelay(); + const absl::Duration never = absl::InfiniteDuration(); + const absl::Duration negative = -absl::InfiniteDuration(); + const absl::Duration immediate = absl::ZeroDuration(); - void Signal(bool v) { - absl::MutexLock lock(&mu_); - b_ = v; - cv_.Signal(); + // Every test case is run twice; once using the absolute deadline API and once + // using the relative timeout API. + std::vector<TimeoutTestParam> values; + for (bool use_absolute_deadline : {false, true}) { + // Tests with a negative timeout (deadline in the past), which should + // immediately return current state of the condition. + + // The condition is already true: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + negative, // wait_timeout + immediate, // satisfy_condition_delay + true, // expected_result + immediate, // expected_delay + }); + + // The condition becomes true, but the timeout has already expired: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + negative, // wait_timeout + finite, // satisfy_condition_delay + false, // expected_result + immediate // expected_delay + }); + + // The condition never becomes true: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + negative, // wait_timeout + never, // satisfy_condition_delay + false, // expected_result + immediate // expected_delay + }); + + // Tests with an infinite timeout (deadline in the infinite future), which + // should only return when the condition becomes true. + + // The condition is already true: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + never, // wait_timeout + immediate, // satisfy_condition_delay + true, // expected_result + immediate // expected_delay + }); + + // The condition becomes true before the (infinite) expiry: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + never, // wait_timeout + finite, // satisfy_condition_delay + true, // expected_result + finite, // expected_delay + }); + + // Tests with a (small) finite timeout (deadline soon), with the condition + // becoming true both before and after its expiry. + + // The condition is already true: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + never, // wait_timeout + immediate, // satisfy_condition_delay + true, // expected_result + immediate // expected_delay + }); + + // The condition becomes true before the expiry: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + finite * 2, // wait_timeout + finite, // satisfy_condition_delay + true, // expected_result + finite // expected_delay + }); + + // The condition becomes true, but the timeout has already expired: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + finite, // wait_timeout + finite * 2, // satisfy_condition_delay + false, // expected_result + finite // expected_delay + }); + + // The condition never becomes true: + values.push_back(TimeoutTestParam{ + __FILE__, __LINE__, use_absolute_deadline, + finite, // wait_timeout + never, // satisfy_condition_delay + false, // expected_result + finite // expected_delay + }); } - - bool WaitWithTimeout(absl::Duration timeout) { - absl::MutexLock lock(&mu_); - absl::Time deadline = absl::Now() + timeout; - if (use_deadline_) { - while (!b_ && !cv_.WaitWithDeadline(&mu_, deadline)) { - } - } else { - while (!b_ && !cv_.WaitWithTimeout(&mu_, timeout)) { - timeout = deadline - absl::Now(); // recompute timeout - } + return values; +} + +// Instantiate `TimeoutTest` with `MakeTimeoutTestParamValues()`. +INSTANTIATE_TEST_CASE_P(All, TimeoutTest, + testing::ValuesIn(MakeTimeoutTestParamValues())); + +TEST_P(TimeoutTest, Await) { + const TimeoutTestParam params = GetParam(); + ABSL_RAW_LOG(INFO, "Params: %s", FormatString(params).c_str()); + + // Because this test asserts bounds on scheduling delays it is flaky. To + // compensate it loops forever until it passes. Failures express as test + // timeouts, in which case the test log can be used to diagnose the issue. + for (int attempt = 1;; ++attempt) { + ABSL_RAW_LOG(INFO, "Attempt %d", attempt); + + absl::Mutex mu; + bool value = false; // condition value (under mu) + + std::unique_ptr<absl::synchronization_internal::ThreadPool> pool = + CreateDefaultPool(); + RunAfterDelay(params.satisfy_condition_delay, pool.get(), [&] { + absl::MutexLock l(&mu); + value = true; + }); + + absl::MutexLock lock(&mu); + absl::Time start_time = absl::Now(); + absl::Condition cond(&value); + bool result = + params.use_absolute_deadline + ? mu.AwaitWithDeadline(cond, start_time + params.wait_timeout) + : mu.AwaitWithTimeout(cond, params.wait_timeout); + if (DelayIsWithinBounds(params.expected_delay, absl::Now() - start_time)) { + EXPECT_EQ(params.expected_result, result); + break; } - return b_; - } - - void Wait() { - absl::MutexLock lock(&mu_); - while (!b_) cv_.Wait(&mu_); - } - - private: - const bool use_deadline_; - - bool b_; - absl::Condition c_; - absl::CondVar cv_; - absl::Mutex mu_; -}; - -class OperationTimer { - public: - OperationTimer() : start_(absl::Now()) {} - absl::Duration Get() const { return absl::Now() - start_; } - - private: - const absl::Time start_; -}; - -static void CheckResults(bool exp_result, bool act_result, - absl::Duration exp_duration, - absl::Duration act_duration) { - ABSL_RAW_CHECK(exp_result == act_result, "CheckResults failed"); - // Allow for some worse-case scheduling delay and clock skew. - if ((exp_duration - absl::Milliseconds(40) > act_duration) || - (exp_duration + absl::Milliseconds(150) < act_duration)) { - ABSL_RAW_LOG(FATAL, "CheckResults failed: operation took %s, expected %s", - absl::FormatDuration(act_duration).c_str(), - absl::FormatDuration(exp_duration).c_str()); } } -static void TestAwaitTimeout(Cond *cp, absl::Duration timeout, bool exp_result, - absl::Duration exp_duration) { - OperationTimer t; - bool act_result = cp->AwaitWithTimeout(timeout); - CheckResults(exp_result, act_result, exp_duration, t.Get()); -} - -static void TestLockWhenTimeout(Cond *cp, absl::Duration timeout, - bool exp_result, absl::Duration exp_duration) { - OperationTimer t; - bool act_result = cp->LockWhenWithTimeout(timeout); - CheckResults(exp_result, act_result, exp_duration, t.Get()); -} +TEST_P(TimeoutTest, LockWhen) { + const TimeoutTestParam params = GetParam(); + ABSL_RAW_LOG(INFO, "Params: %s", FormatString(params).c_str()); + + // Because this test asserts bounds on scheduling delays it is flaky. To + // compensate it loops forever until it passes. Failures express as test + // timeouts, in which case the test log can be used to diagnose the issue. + for (int attempt = 1;; ++attempt) { + ABSL_RAW_LOG(INFO, "Attempt %d", attempt); + + absl::Mutex mu; + bool value = false; // condition value (under mu) + + std::unique_ptr<absl::synchronization_internal::ThreadPool> pool = + CreateDefaultPool(); + RunAfterDelay(params.satisfy_condition_delay, pool.get(), [&] { + absl::MutexLock l(&mu); + value = true; + }); + + absl::Time start_time = absl::Now(); + absl::Condition cond(&value); + bool result = + params.use_absolute_deadline + ? mu.LockWhenWithDeadline(cond, start_time + params.wait_timeout) + : mu.LockWhenWithTimeout(cond, params.wait_timeout); + mu.Unlock(); -static void TestReaderLockWhenTimeout(Cond *cp, absl::Duration timeout, - bool exp_result, - absl::Duration exp_duration) { - OperationTimer t; - bool act_result = cp->ReaderLockWhenWithTimeout(timeout); - CheckResults(exp_result, act_result, exp_duration, t.Get()); + if (DelayIsWithinBounds(params.expected_delay, absl::Now() - start_time)) { + EXPECT_EQ(params.expected_result, result); + break; + } + } } -static void TestWaitTimeout(Cond *cp, absl::Duration timeout, bool exp_result, - absl::Duration exp_duration) { - OperationTimer t; - bool act_result = cp->WaitWithTimeout(timeout); - CheckResults(exp_result, act_result, exp_duration, t.Get()); +TEST_P(TimeoutTest, ReaderLockWhen) { + const TimeoutTestParam params = GetParam(); + ABSL_RAW_LOG(INFO, "Params: %s", FormatString(params).c_str()); + + // Because this test asserts bounds on scheduling delays it is flaky. To + // compensate it loops forever until it passes. Failures express as test + // timeouts, in which case the test log can be used to diagnose the issue. + for (int attempt = 0;; ++attempt) { + ABSL_RAW_LOG(INFO, "Attempt %d", attempt); + + absl::Mutex mu; + bool value = false; // condition value (under mu) + + std::unique_ptr<absl::synchronization_internal::ThreadPool> pool = + CreateDefaultPool(); + RunAfterDelay(params.satisfy_condition_delay, pool.get(), [&] { + absl::MutexLock l(&mu); + value = true; + }); + + absl::Time start_time = absl::Now(); + bool result = + params.use_absolute_deadline + ? mu.ReaderLockWhenWithDeadline(absl::Condition(&value), + start_time + params.wait_timeout) + : mu.ReaderLockWhenWithTimeout(absl::Condition(&value), + params.wait_timeout); + mu.ReaderUnlock(); + + if (DelayIsWithinBounds(params.expected_delay, absl::Now() - start_time)) { + EXPECT_EQ(params.expected_result, result); + break; + } + } } -// Tests with a negative timeout (deadline in the past), which should -// immediately return the current state of the condition. -static void TestNegativeTimeouts(absl::synchronization_internal::ThreadPool *tp, - Cond *cp) { - const absl::Duration negative = -absl::InfiniteDuration(); - const absl::Duration immediate = absl::ZeroDuration(); - - // The condition is already true: - cp->Set(true); - TestAwaitTimeout(cp, negative, true, immediate); - TestLockWhenTimeout(cp, negative, true, immediate); - TestReaderLockWhenTimeout(cp, negative, true, immediate); - TestWaitTimeout(cp, negative, true, immediate); - - // The condition becomes true, but the timeout has already expired: - const absl::Duration delay = absl::Milliseconds(200); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), 3 * delay); - TestAwaitTimeout(cp, negative, false, immediate); - TestLockWhenTimeout(cp, negative, false, immediate); - TestReaderLockWhenTimeout(cp, negative, false, immediate); - cp->Await(); // wait for the scheduled Set() to complete - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Signal, cp, true), delay); - TestWaitTimeout(cp, negative, false, immediate); - cp->Wait(); // wait for the scheduled Signal() to complete - - // The condition never becomes true: - cp->Set(false); - TestAwaitTimeout(cp, negative, false, immediate); - TestLockWhenTimeout(cp, negative, false, immediate); - TestReaderLockWhenTimeout(cp, negative, false, immediate); - TestWaitTimeout(cp, negative, false, immediate); -} - -// Tests with an infinite timeout (deadline in the infinite future), which -// should only return when the condition becomes true. -static void TestInfiniteTimeouts(absl::synchronization_internal::ThreadPool *tp, - Cond *cp) { - const absl::Duration infinite = absl::InfiniteDuration(); - const absl::Duration immediate = absl::ZeroDuration(); - - // The condition is already true: - cp->Set(true); - TestAwaitTimeout(cp, infinite, true, immediate); - TestLockWhenTimeout(cp, infinite, true, immediate); - TestReaderLockWhenTimeout(cp, infinite, true, immediate); - TestWaitTimeout(cp, infinite, true, immediate); - - // The condition becomes true before the (infinite) expiry: - const absl::Duration delay = absl::Milliseconds(200); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), delay); - TestAwaitTimeout(cp, infinite, true, delay); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), delay); - TestLockWhenTimeout(cp, infinite, true, delay); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), delay); - TestReaderLockWhenTimeout(cp, infinite, true, delay); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Signal, cp, true), delay); - TestWaitTimeout(cp, infinite, true, delay); -} - -// Tests with a (small) finite timeout (deadline soon), with the condition -// becoming true both before and after its expiry. -static void TestFiniteTimeouts(absl::synchronization_internal::ThreadPool *tp, - Cond *cp) { - const absl::Duration finite = absl::Milliseconds(400); - const absl::Duration immediate = absl::ZeroDuration(); +TEST_P(TimeoutTest, Wait) { + const TimeoutTestParam params = GetParam(); + ABSL_RAW_LOG(INFO, "Params: %s", FormatString(params).c_str()); + + // Because this test asserts bounds on scheduling delays it is flaky. To + // compensate it loops forever until it passes. Failures express as test + // timeouts, in which case the test log can be used to diagnose the issue. + for (int attempt = 0;; ++attempt) { + ABSL_RAW_LOG(INFO, "Attempt %d", attempt); + + absl::Mutex mu; + bool value = false; // condition value (under mu) + absl::CondVar cv; // signals a change of `value` + + std::unique_ptr<absl::synchronization_internal::ThreadPool> pool = + CreateDefaultPool(); + RunAfterDelay(params.satisfy_condition_delay, pool.get(), [&] { + absl::MutexLock l(&mu); + value = true; + cv.Signal(); + }); + + absl::MutexLock lock(&mu); + absl::Time start_time = absl::Now(); + absl::Duration timeout = params.wait_timeout; + absl::Time deadline = start_time + timeout; + while (!value) { + if (params.use_absolute_deadline ? cv.WaitWithDeadline(&mu, deadline) + : cv.WaitWithTimeout(&mu, timeout)) { + break; // deadline/timeout exceeded + } + timeout = deadline - absl::Now(); // recompute + } + bool result = value; // note: `mu` is still held - // The condition is already true: - cp->Set(true); - TestAwaitTimeout(cp, finite, true, immediate); - TestLockWhenTimeout(cp, finite, true, immediate); - TestReaderLockWhenTimeout(cp, finite, true, immediate); - TestWaitTimeout(cp, finite, true, immediate); - - // The condition becomes true before the expiry: - const absl::Duration delay1 = finite / 2; - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), delay1); - TestAwaitTimeout(cp, finite, true, delay1); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), delay1); - TestLockWhenTimeout(cp, finite, true, delay1); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), delay1); - TestReaderLockWhenTimeout(cp, finite, true, delay1); - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Signal, cp, true), delay1); - TestWaitTimeout(cp, finite, true, delay1); - - // The condition becomes true, but the timeout has already expired: - const absl::Duration delay2 = finite * 2; - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Set, cp, true), 3 * delay2); - TestAwaitTimeout(cp, finite, false, finite); - TestLockWhenTimeout(cp, finite, false, finite); - TestReaderLockWhenTimeout(cp, finite, false, finite); - cp->Await(); // wait for the scheduled Set() to complete - cp->Set(false); - ScheduleAfter(tp, std::bind(&Cond::Signal, cp, true), delay2); - TestWaitTimeout(cp, finite, false, finite); - cp->Wait(); // wait for the scheduled Signal() to complete - - // The condition never becomes true: - cp->Set(false); - TestAwaitTimeout(cp, finite, false, finite); - TestLockWhenTimeout(cp, finite, false, finite); - TestReaderLockWhenTimeout(cp, finite, false, finite); - TestWaitTimeout(cp, finite, false, finite); -} - -TEST(Mutex, Timeouts) { - auto tp = CreateDefaultPool(); - for (bool use_deadline : {false, true}) { - Cond cond(use_deadline); - TestNegativeTimeouts(tp.get(), &cond); - TestInfiniteTimeouts(tp.get(), &cond); - TestFiniteTimeouts(tp.get(), &cond); + if (DelayIsWithinBounds(params.expected_delay, absl::Now() - start_time)) { + EXPECT_EQ(params.expected_result, result); + break; + } } } diff --git a/absl/synchronization/notification.cc b/absl/synchronization/notification.cc index a38acc5f..472b7a3e 100644 --- a/absl/synchronization/notification.cc +++ b/absl/synchronization/notification.cc @@ -22,7 +22,8 @@ #include "absl/time/time.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { + void Notification::Notify() { MutexLock l(&this->mutex_); @@ -82,5 +83,5 @@ bool Notification::WaitForNotificationWithDeadline(absl::Time deadline) const { return notified; } -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl diff --git a/absl/synchronization/notification.h b/absl/synchronization/notification.h index 99f8ee63..25821b18 100644 --- a/absl/synchronization/notification.h +++ b/absl/synchronization/notification.h @@ -52,11 +52,12 @@ #include <atomic> +#include "absl/base/macros.h" #include "absl/synchronization/mutex.h" #include "absl/time/time.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { // ----------------------------------------------------------------------------- // Notification @@ -109,6 +110,6 @@ class Notification { std::atomic<bool> notified_yet_; // written under mutex_ }; -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl #endif // ABSL_SYNCHRONIZATION_NOTIFICATION_H_ diff --git a/absl/synchronization/notification_test.cc b/absl/synchronization/notification_test.cc index 83e17e34..d1b66743 100644 --- a/absl/synchronization/notification_test.cc +++ b/absl/synchronization/notification_test.cc @@ -21,7 +21,7 @@ #include "absl/synchronization/mutex.h" namespace absl { -inline namespace lts_2018_06_20 { +inline namespace lts_2018_12_18 { // A thread-safe class that holds a counter. class ThreadSafeCounter { @@ -72,10 +72,13 @@ static void BasicTests(bool notify_before_waiting, Notification* notification) { notification->WaitForNotificationWithTimeout(absl::Milliseconds(0))); EXPECT_FALSE(notification->WaitForNotificationWithDeadline(absl::Now())); + const absl::Duration delay = absl::Milliseconds(50); + // Allow for a slight early return, to account for quality of implementation + // issues on various platforms. + const absl::Duration slop = absl::Microseconds(200); absl::Time start = absl::Now(); - EXPECT_FALSE( - notification->WaitForNotificationWithTimeout(absl::Milliseconds(50))); - EXPECT_LE(start + absl::Milliseconds(50), absl::Now()); + EXPECT_FALSE(notification->WaitForNotificationWithTimeout(delay)); + EXPECT_LE(start + delay, absl::Now() + slop); ThreadSafeCounter ready_counter; ThreadSafeCounter done_counter; @@ -122,5 +125,5 @@ TEST(NotificationTest, SanityTest) { BasicTests(true, &local_notification2); } -} // inline namespace lts_2018_06_20 +} // inline namespace lts_2018_12_18 } // namespace absl |