Merge #10099: Slightly Improve Unit Tests for Checkqueue
8c2f4b888
Expose more parallelism with relaxed atomics (suggested in #9938). Fix a test to check the exclusive or of two properties rather than just or. (Jeremy Rubin)
Pull request description:
This PR is in response to #10026 and some feedback on #9938.
~Locally, all the checkqueue tests ran 3.2X faster on my machine. The worst offender, `test_CheckQueue_Correct_Random` ran 3.4X faster.~
1. ~Removes `GetRand()` and replaces it with a single deterministic FastRandomContext instance.~ #10321 replicated this
1. Exposes more parallelism with relaxed atomics, increasing chance of catching a bug. This does not change performance on my machine.
1. Makes one test case more restrictive (xor instead of or, see #9938).
Tree-SHA512: a59dfbee0273c713525a130dfedc1c7ff26f50c2aaca1e94ef5d759b1d6ea6338ffbd97f863b9f6209750d8a788a15fa8ae1bf26774ed2473c520811337e6b00
This commit is contained in:
commit
424be03305
1 changed files with 18 additions and 15 deletions
|
@ -38,7 +38,7 @@ struct FakeCheckCheckCompletion {
|
|||
static std::atomic<size_t> n_calls;
|
||||
bool operator()()
|
||||
{
|
||||
++n_calls;
|
||||
n_calls.fetch_add(1, std::memory_order_relaxed);
|
||||
return true;
|
||||
}
|
||||
void swap(FakeCheckCheckCompletion& x){};
|
||||
|
@ -88,15 +88,15 @@ struct MemoryCheck {
|
|||
//
|
||||
// Really, copy constructor should be deletable, but CCheckQueue breaks
|
||||
// if it is deleted because of internal push_back.
|
||||
fake_allocated_memory += b;
|
||||
fake_allocated_memory.fetch_add(b, std::memory_order_relaxed);
|
||||
};
|
||||
MemoryCheck(bool b_) : b(b_)
|
||||
{
|
||||
fake_allocated_memory += b;
|
||||
fake_allocated_memory.fetch_add(b, std::memory_order_relaxed);
|
||||
};
|
||||
~MemoryCheck(){
|
||||
fake_allocated_memory -= b;
|
||||
|
||||
~MemoryCheck()
|
||||
{
|
||||
fake_allocated_memory.fetch_sub(b, std::memory_order_relaxed);
|
||||
};
|
||||
void swap(MemoryCheck& x) { std::swap(b, x.b); };
|
||||
};
|
||||
|
@ -117,9 +117,9 @@ struct FrozenCleanupCheck {
|
|||
{
|
||||
if (should_freeze) {
|
||||
std::unique_lock<std::mutex> l(m);
|
||||
nFrozen = 1;
|
||||
nFrozen.store(1, std::memory_order_relaxed);
|
||||
cv.notify_one();
|
||||
cv.wait(l, []{ return nFrozen == 0;});
|
||||
cv.wait(l, []{ return nFrozen.load(std::memory_order_relaxed) == 0;});
|
||||
}
|
||||
}
|
||||
void swap(FrozenCleanupCheck& x){std::swap(should_freeze, x.should_freeze);};
|
||||
|
@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure)
|
|||
control.Add(vChecks);
|
||||
}
|
||||
bool r =control.Wait();
|
||||
BOOST_REQUIRE(r || end_fails);
|
||||
BOOST_REQUIRE(r != end_fails);
|
||||
}
|
||||
}
|
||||
tg.interrupt_all();
|
||||
|
@ -361,11 +361,14 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup)
|
|||
std::unique_lock<std::mutex> l(FrozenCleanupCheck::m);
|
||||
// Wait until the queue has finished all jobs and frozen
|
||||
FrozenCleanupCheck::cv.wait(l, [](){return FrozenCleanupCheck::nFrozen == 1;});
|
||||
// Try to get control of the queue a bunch of times
|
||||
for (auto x = 0; x < 100 && !fails; ++x) {
|
||||
fails = queue->ControlMutex.try_lock();
|
||||
}
|
||||
// Unfreeze
|
||||
}
|
||||
// Try to get control of the queue a bunch of times
|
||||
for (auto x = 0; x < 100 && !fails; ++x) {
|
||||
fails = queue->ControlMutex.try_lock();
|
||||
}
|
||||
{
|
||||
// Unfreeze (we need lock n case of spurious wakeup)
|
||||
std::unique_lock<std::mutex> l(FrozenCleanupCheck::m);
|
||||
FrozenCleanupCheck::nFrozen = 0;
|
||||
}
|
||||
// Awaken frozen destructor
|
||||
|
|
Loading…
Reference in a new issue