From 58335c4bd06216548f4a69af8282d9d017648055 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 May 2016 19:02:38 -0700 Subject: [PATCH 1/4] Use optimistic behavior in Barrier This eliminates one lock acquisition during Barrier and implements a double-check pattern, which makes the Barrier routine significantly faster in cases where the queue is empty. --- src/autowiring/DispatchQueue.cpp | 12 ++++++++---- src/autowiring/DispatchQueue.h | 4 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index a5339f9fe..a9802752a 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -280,6 +280,14 @@ void DispatchQueue::PendExisting(std::unique_lock&& lk, DispatchThun bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { // Optimistic check first: + if (!m_count) + return true; + + // Short-circuit if zero is specified as the timeout value + if (timeout.count() == 0) + return false; + + // Now lock and double-check: std::unique_lock lk(m_dispatchLock); // Short-circuit if dispatching has been aborted @@ -290,10 +298,6 @@ bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { if (!m_count) return true; - // Also short-circuit if zero is specified as the timeout value - if (timeout.count() == 0) - return false; - // Set up the lambda. Note that the queue size CANNOT be 1, because we just checked to verify // that it is non-empty. Thus, we do not need to signal the m_queueUpdated condition variable. auto complete = std::make_shared(false); diff --git a/src/autowiring/DispatchQueue.h b/src/autowiring/DispatchQueue.h index 94f0b8a27..096756c0b 100644 --- a/src/autowiring/DispatchQueue.h +++ b/src/autowiring/DispatchQueue.h @@ -247,7 +247,9 @@ class DispatchQueue { /// /// Blocks until all dispatchers on the DispatchQueue at the time of the call have been dispatched /// - /// The maximum amount of time to wait + /// + /// The maximum amount of time to wait. If this value is zero, this method will not obtain any locks. + /// /// /// This method does not cause any dispatchers to run. If the underlying dispatch queue does not have an event loop /// operating on it, this method will deadlock. It is an error for the party responsible for driving the dispatch queue From c0775e0487c0d53c3369ade5086a653c93262833 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 23 May 2016 10:58:22 -0700 Subject: [PATCH 2/4] Adjustment of Barrier behavior to make it similar to other overload --- src/autowiring/DispatchQueue.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index a9802752a..b18b9e09e 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -279,13 +279,9 @@ void DispatchQueue::PendExisting(std::unique_lock&& lk, DispatchThun } bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { - // Optimistic check first: - if (!m_count) - return true; - // Short-circuit if zero is specified as the timeout value - if (timeout.count() == 0) - return false; + if (!m_count && timeout.count() == 0) + return true; // Now lock and double-check: std::unique_lock lk(m_dispatchLock); @@ -294,10 +290,6 @@ bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { if (onAborted) throw dispatch_aborted_exception("Dispatch queue was aborted before a timed wait was attempted"); - // Short-circuit if the queue is already empty - if (!m_count) - return true; - // Set up the lambda. Note that the queue size CANNOT be 1, because we just checked to verify // that it is non-empty. Thus, we do not need to signal the m_queueUpdated condition variable. auto complete = std::make_shared(false); From f0929e3c97cfb582def6a740719731efd71b6250 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 23 May 2016 12:08:05 -0700 Subject: [PATCH 3/4] If the timeout is zero, do not acquire any locks whatsoever --- src/autowiring/DispatchQueue.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index b18b9e09e..5bf9bfb6b 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -279,9 +279,9 @@ void DispatchQueue::PendExisting(std::unique_lock&& lk, DispatchThun } bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { - // Short-circuit if zero is specified as the timeout value - if (!m_count && timeout.count() == 0) - return true; + // Do not block or lock in the event of a no-wait check + if (timeout.count() == 0) + return m_count == 0; // Now lock and double-check: std::unique_lock lk(m_dispatchLock); From c9988a0be5d9251cdd902aca95c77f1c893a6291 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 23 May 2016 12:24:06 -0700 Subject: [PATCH 4/4] Fix comment language --- src/autowiring/DispatchQueue.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/autowiring/DispatchQueue.h b/src/autowiring/DispatchQueue.h index 096756c0b..8e409b1e0 100644 --- a/src/autowiring/DispatchQueue.h +++ b/src/autowiring/DispatchQueue.h @@ -248,18 +248,23 @@ class DispatchQueue { /// Blocks until all dispatchers on the DispatchQueue at the time of the call have been dispatched /// /// - /// The maximum amount of time to wait. If this value is zero, this method will not obtain any locks. + /// The maximum amount of time to wait. If this value is zero, this method will not wait. /// /// /// This method does not cause any dispatchers to run. If the underlying dispatch queue does not have an event loop /// operating on it, this method will deadlock. It is an error for the party responsible for driving the dispatch queue - /// via WaitForEvent or DispatchAllEvents unless that party first delegates the responsibility elsewhere. + /// via WaitForEvent or DispatchAllEvents to call this method unless that party first delegates the responsibility + /// elsewhere. /// /// If DispatchQueue::Abort() is called before the dispatcher has been completed, this method will throw an exception. /// If a dispatcher on the underlying DispatchQueue throws an exception, this method will also throw an exception. /// /// If zero is passed as the timeout value, this method will return true if and only if the queue was empty at the time /// of the call, ignoring any delayed dispatchers. + /// + /// If timeout is nonzero, this method will pend a dispatcher to the queue and only return true if this dispatcher + /// is actually dispatched. If the timeout is zero, this method will return true immediately if the queue length + /// is exactly zero. /// bool Barrier(std::chrono::nanoseconds timeout);