- Revision
- 188374
- Author
- [email protected]
- Date
- 2015-08-12 20:51:25 -0700 (Wed, 12 Aug 2015)
Log Message
WTF::Lock should not suffer from the thundering herd
https://bugs.webkit.org/show_bug.cgi?id=147947
Reviewed by Geoffrey Garen.
Source/WTF:
This changes Lock::unlockSlow() to use unparkOne() instead of unparkAll(). The problem with
doing this is that it's not obvious after calling unparkOne() if there are any other threads
that are still parked on the lock's queue. If we assume that there are and leave the
hasParkedBit set, then future calls to unlock() will take the slow path. We don't want that
if there aren't actually any threads parked. On the other hand, if we assume that there
aren't any threads parked and clear the hasParkedBit, then if there actually were some
threads parked, then they may never be awoken since future calls to unlock() won't take slow
path and so won't call unparkOne(). In other words, we need a way to be very precise about
when we clear the hasParkedBit and we need to do it in a race-free way: it can't be the case
that we clear the bit just as some thread gets parked on the queue.
A similar problem arises in futexes, and one of the solutions is to have a thread that
acquires a lock after parking sets the hasParkedBit. This is what Rusty Russel's usersem
does. It's a subtle algorithm. Also, it means that if a thread barges in before the unparked
thread runs, then that barging thread will not know that there are threads parked. This
could increase the severity of barging.
Since ParkingLot is a user-level API, we don't have to worry about the kernel-user security
issues and so we can expose callbacks while ParkingLot is holding its internal locks. This
change does exactly that for unparkOne(). The new variant of unparkOne() will call a user
function while the queue from which we are unparking is locked. The callback is told basic
stats about the queue: did we unpark a thread this time, and could there be more threads to
unpark in the future. The callback runs while it's impossible for the queue state to change,
since the ParkingLot's internal locks for the queue is held. This means that
Lock::unlockSlow() can either clear, or leave, the hasParkedBit while releasing the lock
inside the callback from unparkOne(). This takes care of the thundering herd problem while
also reducing the greed that arises from barging threads.
This required some careful reworking of the ParkingLot algorithm. The first thing I noticed
was that the ThreadData::shouldPark flag was useless, since it's set exactly when
ThreadData::address is non-null. Then I had to make sure that dequeue() could lazily create
both hashtables and buckets, since the "callback is called while queue is locked" invariant
requires that we didn't exit early due to the hashtable or bucket not being present. Note
that all of this is done in such a way that the old unparkOne() and unparkAll() don't have
to create any buckets, though they now may create the hashtable. We don't care as much about
the hashtable being created by unpark since it's just such an unlikely scenario and it would
only happen once.
This change reduces the kernel CPU usage of WTF::Lock for the long critical section test by
about 8x and makes it always perform as well as WTF::WordLock and WTF::Mutex for that
benchmark.
* benchmarks/LockSpeedTest.cpp:
* wtf/Lock.cpp:
(WTF::LockBase::unlockSlow):
* wtf/Lock.h:
(WTF::LockBase::isLocked):
(WTF::LockBase::isFullyReset):
* wtf/ParkingLot.cpp:
(WTF::ParkingLot::parkConditionally):
(WTF::ParkingLot::unparkOne):
(WTF::ParkingLot::unparkAll):
* wtf/ParkingLot.h:
* wtf/WordLock.h:
(WTF::WordLock::isLocked):
(WTF::WordLock::isFullyReset):
Tools:
Add testing that checks that locks return to a pristine state after contention is over.
* TestWebKitAPI/Tests/WTF/Lock.cpp:
(TestWebKitAPI::LockInspector::isFullyReset):
(TestWebKitAPI::runLockTest):
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (188373 => 188374)
--- trunk/Source/WTF/ChangeLog 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/ChangeLog 2015-08-13 03:51:25 UTC (rev 188374)
@@ -1,3 +1,67 @@
+2015-08-12 Filip Pizlo <[email protected]>
+
+ WTF::Lock should not suffer from the thundering herd
+ https://bugs.webkit.org/show_bug.cgi?id=147947
+
+ Reviewed by Geoffrey Garen.
+
+ This changes Lock::unlockSlow() to use unparkOne() instead of unparkAll(). The problem with
+ doing this is that it's not obvious after calling unparkOne() if there are any other threads
+ that are still parked on the lock's queue. If we assume that there are and leave the
+ hasParkedBit set, then future calls to unlock() will take the slow path. We don't want that
+ if there aren't actually any threads parked. On the other hand, if we assume that there
+ aren't any threads parked and clear the hasParkedBit, then if there actually were some
+ threads parked, then they may never be awoken since future calls to unlock() won't take slow
+ path and so won't call unparkOne(). In other words, we need a way to be very precise about
+ when we clear the hasParkedBit and we need to do it in a race-free way: it can't be the case
+ that we clear the bit just as some thread gets parked on the queue.
+
+ A similar problem arises in futexes, and one of the solutions is to have a thread that
+ acquires a lock after parking sets the hasParkedBit. This is what Rusty Russel's usersem
+ does. It's a subtle algorithm. Also, it means that if a thread barges in before the unparked
+ thread runs, then that barging thread will not know that there are threads parked. This
+ could increase the severity of barging.
+
+ Since ParkingLot is a user-level API, we don't have to worry about the kernel-user security
+ issues and so we can expose callbacks while ParkingLot is holding its internal locks. This
+ change does exactly that for unparkOne(). The new variant of unparkOne() will call a user
+ function while the queue from which we are unparking is locked. The callback is told basic
+ stats about the queue: did we unpark a thread this time, and could there be more threads to
+ unpark in the future. The callback runs while it's impossible for the queue state to change,
+ since the ParkingLot's internal locks for the queue is held. This means that
+ Lock::unlockSlow() can either clear, or leave, the hasParkedBit while releasing the lock
+ inside the callback from unparkOne(). This takes care of the thundering herd problem while
+ also reducing the greed that arises from barging threads.
+
+ This required some careful reworking of the ParkingLot algorithm. The first thing I noticed
+ was that the ThreadData::shouldPark flag was useless, since it's set exactly when
+ ThreadData::address is non-null. Then I had to make sure that dequeue() could lazily create
+ both hashtables and buckets, since the "callback is called while queue is locked" invariant
+ requires that we didn't exit early due to the hashtable or bucket not being present. Note
+ that all of this is done in such a way that the old unparkOne() and unparkAll() don't have
+ to create any buckets, though they now may create the hashtable. We don't care as much about
+ the hashtable being created by unpark since it's just such an unlikely scenario and it would
+ only happen once.
+
+ This change reduces the kernel CPU usage of WTF::Lock for the long critical section test by
+ about 8x and makes it always perform as well as WTF::WordLock and WTF::Mutex for that
+ benchmark.
+
+ * benchmarks/LockSpeedTest.cpp:
+ * wtf/Lock.cpp:
+ (WTF::LockBase::unlockSlow):
+ * wtf/Lock.h:
+ (WTF::LockBase::isLocked):
+ (WTF::LockBase::isFullyReset):
+ * wtf/ParkingLot.cpp:
+ (WTF::ParkingLot::parkConditionally):
+ (WTF::ParkingLot::unparkOne):
+ (WTF::ParkingLot::unparkAll):
+ * wtf/ParkingLot.h:
+ * wtf/WordLock.h:
+ (WTF::WordLock::isLocked):
+ (WTF::WordLock::isFullyReset):
+
2015-08-11 Filip Pizlo <[email protected]>
Always use a byte-sized lock implementation
Modified: trunk/Source/WTF/benchmarks/LockSpeedTest.cpp (188373 => 188374)
--- trunk/Source/WTF/benchmarks/LockSpeedTest.cpp 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/benchmarks/LockSpeedTest.cpp 2015-08-13 03:51:25 UTC (rev 188374)
@@ -44,7 +44,7 @@
NO_RETURN void usage()
{
- printf("Usage: LockSpeedTest spinlock|wordlock|lock|bytelock|mutex|all <num thread groups> <num threads per group> <work per critical section> <num noise threads> <num iterations>\n");
+ printf("Usage: LockSpeedTest spinlock|wordlock|lock|mutex|all <num thread groups> <num threads per group> <work per critical section> <num noise threads> <num iterations>\n");
exit(1);
}
Modified: trunk/Source/WTF/wtf/Lock.cpp (188373 => 188374)
--- trunk/Source/WTF/wtf/Lock.cpp 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/wtf/Lock.cpp 2015-08-13 03:51:25 UTC (rev 188374)
@@ -69,37 +69,43 @@
// We now expect the value to be isHeld|hasParked. So long as that's the case, we can park.
ParkingLot::compareAndPark(&m_byte, isHeldBit | hasParkedBit);
- // We have awaken, or we never parked because the byte value changed. Either way, we loop
+ // We have awoken, or we never parked because the byte value changed. Either way, we loop
// around and try again.
}
}
void LockBase::unlockSlow()
{
- // Release the lock while finding out if someone is parked. Note that as soon as we do this,
- // someone might barge in.
- uint8_t oldByteValue;
+ // We could get here because the weak CAS in unlock() failed spuriously, or because there is
+ // someone parked. So, we need a CAS loop: even if right now the lock is just held, it could
+ // be held and parked if someone attempts to lock just as we are unlocking.
for (;;) {
- oldByteValue = m_byte.load();
- if (verbose)
- dataLog(toString(currentThread(), ": unlocking with ", oldByteValue, "\n"));
- ASSERT(oldByteValue & isHeldBit);
- if (m_byte.compareExchangeWeak(oldByteValue, 0))
- break;
- }
+ uint8_t oldByteValue = m_byte.load();
+ ASSERT(oldByteValue == isHeldBit || oldByteValue == (isHeldBit | hasParkedBit));
+
+ if (oldByteValue == isHeldBit) {
+ if (m_byte.compareExchangeWeak(isHeldBit, 0))
+ return;
+ continue;
+ }
- // Note that someone could try to park right now. If that happens, they will return immediately
- // because our parking predicate is that m_byte == isHeldBit | hasParkedBit, but we've already set
- // m_byte = 0.
+ // Someone is parked. Unpark exactly one thread, possibly leaving the parked bit set if
+ // there is a chance that there are still other threads parked.
+ ASSERT(oldByteValue == (isHeldBit | hasParkedBit));
+ ParkingLot::unparkOne(
+ &m_byte,
+ [this] (bool, bool mayHaveMoreThreads) {
+ // We are the only ones that can clear either the isHeldBit or the hasParkedBit,
+ // so we should still see both bits set right now.
+ ASSERT(m_byte.load() == (isHeldBit | hasParkedBit));
- // If there had been threads parked, unpark all of them. This causes a thundering herd, but while
- // that is theoretically scary, it's totally fine in WebKit because we usually don't have enough
- // threads for this to matter.
- // FIXME: We don't really need this to exhibit thundering herd. We could use unparkOne(), and if
- // that returns true, just set the parked bit again. If in the process of setting the parked bit
- // we fail the CAS, then just unpark again.
- if (oldByteValue & hasParkedBit)
- ParkingLot::unparkAll(&m_byte);
+ if (mayHaveMoreThreads)
+ m_byte.store(hasParkedBit);
+ else
+ m_byte.store(0);
+ });
+ return;
+ }
}
} // namespace WTF
Modified: trunk/Source/WTF/wtf/Lock.h (188373 => 188374)
--- trunk/Source/WTF/wtf/Lock.h 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/wtf/Lock.h 2015-08-13 03:51:25 UTC (rev 188374)
@@ -31,6 +31,10 @@
#include <wtf/Locker.h>
#include <wtf/Noncopyable.h>
+namespace TestWebKitAPI {
+struct LockInspector;
+};
+
namespace WTF {
// This is a fully adaptive mutex that only requires 1 byte of storage. It has fast paths that are
@@ -73,12 +77,20 @@
}
protected:
+ friend struct TestWebKitAPI::LockInspector;
+
static const uint8_t isHeldBit = 1;
static const uint8_t hasParkedBit = 2;
WTF_EXPORT_PRIVATE void lockSlow();
WTF_EXPORT_PRIVATE void unlockSlow();
+ // Method used for testing only.
+ bool isFullyReset() const
+ {
+ return !m_byte.load();
+ }
+
Atomic<uint8_t> m_byte;
};
Modified: trunk/Source/WTF/wtf/ParkingLot.cpp (188373 => 188374)
--- trunk/Source/WTF/wtf/ParkingLot.cpp 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/wtf/ParkingLot.cpp 2015-08-13 03:51:25 UTC (rev 188374)
@@ -52,11 +52,11 @@
ThreadIdentifier threadIdentifier;
- bool shouldPark { false };
std::mutex parkingLock;
std::condition_variable parkingCondition;
- const void* address;
+ const void* address { nullptr };
+
ThreadData* nextInQueue { nullptr };
};
@@ -212,26 +212,37 @@
return WTF::PtrHash<const void*>::hash(address);
}
-// Locks the hashtable. This reloops in case of rehashing, so the current hashtable may be different
-// after this returns than when you called it. Guarantees that there is a hashtable. This is pretty
-// slow and not scalable, so it's only used during thread creation and for debugging/testing.
-Vector<Bucket*> lockHashtable()
+Hashtable* ensureHashtable()
{
for (;;) {
- Hashtable* currentHashtable;
+ Hashtable* currentHashtable = hashtable.load();
- currentHashtable = hashtable.load();
+ if (currentHashtable)
+ return currentHashtable;
+
if (!currentHashtable) {
- // Try to be the first to create the hashtable.
currentHashtable = Hashtable::create(maxLoadFactor);
- if (!hashtable.compareExchangeWeak(nullptr, currentHashtable)) {
- Hashtable::destroy(currentHashtable);
- continue;
+ if (hashtable.compareExchangeWeak(nullptr, currentHashtable)) {
+ if (verbose)
+ dataLog(toString(currentThread(), ": created initial hashtable ", RawPointer(currentHashtable), "\n"));
+ return currentHashtable;
}
- if (verbose)
- dataLog(toString(currentThread(), ": created initial hashtable ", RawPointer(currentHashtable), "\n"));
+
+ Hashtable::destroy(currentHashtable);
}
+ }
+}
+// Locks the hashtable. This reloops in case of rehashing, so the current hashtable may be different
+// after this returns than when you called it. Guarantees that there is a hashtable. This is pretty
+// slow and not scalable, so it's only used during thread creation and for debugging/testing.
+Vector<Bucket*> lockHashtable()
+{
+ for (;;) {
+ Hashtable* currentHashtable = ensureHashtable();
+
+ ASSERT(currentHashtable);
+
// Now find all of the buckets. This makes sure that the hashtable is full of buckets so that
// we can lock all of the buckets, not just the ones that are materialized.
Vector<Bucket*> buckets;
@@ -405,7 +416,7 @@
unsigned hash = hashAddress(address);
for (;;) {
- Hashtable* myHashtable = hashtable.load();
+ Hashtable* myHashtable = ensureHashtable();
unsigned index = hash % myHashtable->size;
Atomic<Bucket*>& bucketPointer = myHashtable->data[index];
Bucket* bucket;
@@ -444,50 +455,51 @@
}
}
-template<typename Functor>
-bool dequeue(const void* address, const Functor& functor)
+enum class BucketMode {
+ EnsureNonEmpty,
+ IgnoreEmpty
+};
+
+template<typename DequeueFunctor, typename FinishFunctor>
+bool dequeue(
+ const void* address, BucketMode bucketMode, const DequeueFunctor& dequeueFunctor,
+ const FinishFunctor& finishFunctor)
{
- if (verbose)
- dataLog(toString(currentThread(), ": dequeueing address ", RawPointer(address), "\n"));
unsigned hash = hashAddress(address);
- if (verbose)
- dataLog(toString(currentThread(), ": hash = ", hash, "\n"));
for (;;) {
- Hashtable* myHashtable = hashtable.load();
- if (!myHashtable) {
- if (verbose)
- dataLog(toString(currentThread(), ": no hashtable.\n"));
- return false;
- }
+ Hashtable* myHashtable = ensureHashtable();
unsigned index = hash % myHashtable->size;
- if (verbose)
- dataLog(toString(currentThread(), ": index = ", index, "\n"));
- Bucket* bucket = myHashtable->data[index].load();
+ Atomic<Bucket*>& bucketPointer = myHashtable->data[index];
+ Bucket* bucket = bucketPointer.load();
if (!bucket) {
- if (verbose)
- dataLog(toString(currentThread(), ": no bucket at ", RawPointer(bucket), ".\n"));
- return false;
+ if (bucketMode == BucketMode::IgnoreEmpty)
+ return false;
+
+ for (;;) {
+ bucket = bucketPointer.load();
+ if (!bucket) {
+ bucket = new Bucket();
+ if (!bucketPointer.compareExchangeWeak(nullptr, bucket)) {
+ delete bucket;
+ continue;
+ }
+ }
+ break;
+ }
}
- if (verbose)
- dataLog(toString(currentThread(), ": locking bucket at ", RawPointer(bucket), "\n"));
bucket->lock.lock();
- if (verbose)
- dataLog(toString(currentThread(), ": locked bucket at ", RawPointer(bucket), "\n"));
// At this point the hashtable could have rehashed under us.
if (hashtable.load() != myHashtable) {
- if (verbose)
- dataLog(toString(currentThread(), ": hashtable changed.\n"));
bucket->lock.unlock();
continue;
}
- if (verbose)
- dataLog(toString(currentThread(), ": found bucket.\n"));
- bucket->genericDequeue(functor);
+ bucket->genericDequeue(dequeueFunctor);
bool result = !!bucket->queueHead;
+ finishFunctor(result);
bucket->lock.unlock();
return result;
}
@@ -502,8 +514,6 @@
ThreadData* me = myThreadData();
- ASSERT(!me->shouldPark);
-
bool result = enqueue(
address,
[&] () -> ThreadData* {
@@ -511,7 +521,6 @@
return nullptr;
me->address = address;
- me->shouldPark = true;
return me;
});
@@ -522,10 +531,8 @@
dataLog(toString(currentThread(), ": parking self: ", RawPointer(me), "\n"));
{
std::unique_lock<std::mutex> locker(me->parkingLock);
- while (me->shouldPark)
+ while (me->address)
me->parkingCondition.wait(locker);
-
- me->address = nullptr;
}
if (verbose)
dataLog(toString(currentThread(), ": unparked self: ", RawPointer(me), "\n"));
@@ -540,27 +547,62 @@
ThreadData* threadData = nullptr;
bool result = dequeue(
address,
+ BucketMode::IgnoreEmpty,
[&] (ThreadData* element) {
if (element->address != address)
return DequeueResult::Ignore;
threadData = element;
return DequeueResult::RemoveAndStop;
- });
+ },
+ [] (bool) { });
if (!threadData)
return false;
- ASSERT(threadData->shouldPark);
+ ASSERT(threadData->address);
{
std::unique_lock<std::mutex> locker(threadData->parkingLock);
- threadData->shouldPark = false;
- threadData->parkingCondition.notify_all();
+ threadData->address = nullptr;
}
+ threadData->parkingCondition.notify_one();
return result;
}
+void ParkingLot::unparkOne(
+ const void* address,
+ std::function<void(bool didUnparkThread, bool mayHaveMoreThreads)> callback)
+{
+ if (verbose)
+ dataLog(toString(currentThread(), ": unparking one the hard way.\n"));
+
+ ThreadData* threadData = nullptr;
+ dequeue(
+ address,
+ BucketMode::EnsureNonEmpty,
+ [&] (ThreadData* element) {
+ if (element->address != address)
+ return DequeueResult::Ignore;
+ threadData = element;
+ return DequeueResult::RemoveAndStop;
+ },
+ [&] (bool mayHaveMoreThreads) {
+ callback(!!threadData, threadData && mayHaveMoreThreads);
+ });
+
+ if (!threadData)
+ return;
+
+ ASSERT(threadData->address);
+
+ {
+ std::unique_lock<std::mutex> locker(threadData->parkingLock);
+ threadData->address = nullptr;
+ }
+ threadData->parkingCondition.notify_one();
+}
+
void ParkingLot::unparkAll(const void* address)
{
if (verbose)
@@ -569,6 +611,7 @@
Vector<ThreadData*, 8> threadDatas;
dequeue(
address,
+ BucketMode::IgnoreEmpty,
[&] (ThreadData* element) {
if (verbose)
dataLog(toString(currentThread(), ": Observing element with address = ", RawPointer(element->address), "\n"));
@@ -576,15 +619,18 @@
return DequeueResult::Ignore;
threadDatas.append(element);
return DequeueResult::RemoveAndContinue;
- });
+ },
+ [] (bool) { });
for (ThreadData* threadData : threadDatas) {
if (verbose)
dataLog(toString(currentThread(), ": unparking ", RawPointer(threadData), " with address ", RawPointer(threadData->address), "\n"));
- ASSERT(threadData->shouldPark);
- std::unique_lock<std::mutex> locker(threadData->parkingLock);
- threadData->shouldPark = false;
- threadData->parkingCondition.notify_all();
+ ASSERT(threadData->address);
+ {
+ std::unique_lock<std::mutex> locker(threadData->parkingLock);
+ threadData->address = nullptr;
+ }
+ threadData->parkingCondition.notify_one();
}
if (verbose)
Modified: trunk/Source/WTF/wtf/ParkingLot.h (188373 => 188374)
--- trunk/Source/WTF/wtf/ParkingLot.h 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/wtf/ParkingLot.h 2015-08-13 03:51:25 UTC (rev 188374)
@@ -59,6 +59,21 @@
// are no more threads on the queue.
WTF_EXPORT_PRIVATE static bool unparkOne(const void* address);
+ // Unparks one thread from the queue associated with the given address, and calls the given
+ // functor while the address is locked. Reports to the callback whether any thread got unparked
+ // and whether there may be any other threads still on the queue. This is an expert-mode version
+ // of unparkOne() that allows for really good thundering herd avoidance in adaptive mutexes.
+ // Without this, a lock implementation that uses unparkOne() has to have some trick for knowing
+ // if there are still threads parked on the queue, so that it can set some bit in its lock word
+ // to indicate that the next unlock() also needs to unparkOne(). But there is a race between
+ // manipulating that bit and some other thread acquiring the lock. It's possible to work around
+ // that race - see Rusty Russel's well-known usersem library - but it's not pretty. This form
+ // allows that race to be completely avoided, since there is no way that a thread can be parked
+ // while the callback is running.
+ WTF_EXPORT_PRIVATE static void unparkOne(
+ const void* address,
+ std::function<void(bool didUnparkThread, bool mayHaveMoreThreads)> callback);
+
// Unparks every thread from the queue associated with the given address, which cannot be null.
WTF_EXPORT_PRIVATE static void unparkAll(const void* address);
Modified: trunk/Source/WTF/wtf/WordLock.h (188373 => 188374)
--- trunk/Source/WTF/wtf/WordLock.h 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Source/WTF/wtf/WordLock.h 2015-08-13 03:51:25 UTC (rev 188374)
@@ -31,6 +31,10 @@
#include <wtf/Locker.h>
#include <wtf/Noncopyable.h>
+namespace TestWebKitAPI {
+struct LockInspector;
+};
+
namespace WTF {
// A WordLock is a fully adaptive mutex that uses sizeof(void*) storage. It has a fast path that is
@@ -77,6 +81,8 @@
}
private:
+ friend struct TestWebKitAPI::LockInspector;
+
static const uintptr_t isLockedBit = 1;
static const uintptr_t isQueueLockedBit = 2;
static const uintptr_t queueHeadMask = 3;
@@ -84,6 +90,12 @@
WTF_EXPORT_PRIVATE void lockSlow();
WTF_EXPORT_PRIVATE void unlockSlow();
+ // Method used for testing only.
+ bool isFullyReset() const
+ {
+ return !m_word.load();
+ }
+
Atomic<uintptr_t> m_word;
};
Modified: trunk/Tools/ChangeLog (188373 => 188374)
--- trunk/Tools/ChangeLog 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Tools/ChangeLog 2015-08-13 03:51:25 UTC (rev 188374)
@@ -1,3 +1,17 @@
+2015-08-12 Filip Pizlo <[email protected]>
+
+ WTF::Lock should not suffer from the thundering herd
+ https://bugs.webkit.org/show_bug.cgi?id=147947
+
+ Reviewed by Geoffrey Garen.
+
+ Add testing that checks that locks return to a pristine state after contention is over.
+
+ * TestWebKitAPI/Tests/WTF/Lock.cpp:
+ (TestWebKitAPI::LockInspector::isFullyReset):
+ (TestWebKitAPI::runLockTest):
+ (TestWebKitAPI::TEST):
+
2015-08-12 Dewei Zhu <[email protected]>
Benchmarks supported by run_benchmark script should not assume we have internet access.
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/Lock.cpp (188373 => 188374)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/Lock.cpp 2015-08-13 02:12:13 UTC (rev 188373)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/Lock.cpp 2015-08-13 03:51:25 UTC (rev 188374)
@@ -33,6 +33,14 @@
namespace TestWebKitAPI {
+struct LockInspector {
+ template<typename LockType>
+ static bool isFullyReset(LockType& lock)
+ {
+ return lock.isFullyReset();
+ }
+};
+
template<typename LockType>
void runLockTest(unsigned numThreadGroups, unsigned numThreadsPerGroup, unsigned workPerCriticalSection, unsigned numIterations)
{
@@ -66,6 +74,17 @@
for (unsigned threadGroupIndex = numThreadGroups; threadGroupIndex--;)
EXPECT_EQ(expected, words[threadGroupIndex]);
+
+ // Now test that the locks correctly reset themselves. We expect that if a single thread locks
+ // each of the locks twice in a row, then the lock should be in a pristine state.
+ for (unsigned threadGroupIndex = numThreadGroups; threadGroupIndex--;) {
+ for (unsigned i = 2; i--;) {
+ locks[threadGroupIndex].lock();
+ locks[threadGroupIndex].unlock();
+ }
+
+ EXPECT_EQ(true, LockInspector::isFullyReset(locks[threadGroupIndex]));
+ }
}
TEST(WTF_WordLock, UncontendedShortSection)
@@ -128,6 +147,11 @@
runLockTest<Lock>(10, 10, 10000, 1000);
}
+TEST(WTF_Lock, ManyContendedLongerSections)
+{
+ runLockTest<Lock>(10, 10, 100000, 100);
+}
+
TEST(WTF_Lock, SectionAddressCollision)
{
runLockTest<Lock>(4, 2, 10000, 2000);
