Skip to content

Commit

Permalink
Merge bitcoin#23397: Avoid excessive lock contention in CCheckQueue::Add
Browse files Browse the repository at this point in the history
459e208 Exit early for an empty vChecks in CCheckQueue::Add (Hennadii Stepanov)
c43aa62 Avoid excessive lock contention in CCheckQueue::Add (Hennadii Stepanov)

Pull request description:

  This PR significantly reduces lock contention in the `CCheckQueue` class by releasing a mutex before calling `std::condition_variable::notify_one` and `std::condition_variable::notify_all`.

  From C++ [docs](https://en.cppreference.com/w/cpp/thread/condition_variable/notify_one):
  > The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s); in fact doing so is a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock.

  Related to:
  - bitcoin#23167
  - bitcoin#23223

ACKs for top commit:
  martinus:
    ACK 459e208, codereview and tested. I first thought this introduced a segfault in `psbt_wallet_tests/psbt_updater_test` because that test failed for me, but thats a different issue fixed in bitcoin#23403.
  vasild:
    ACK 459e208
  theStack:
    Code-review ACK 459e208

Tree-SHA512: c197858656392ba3ebcd638d713cf93c9fb48b7b3bad193209490d2828f9c7e3ae4dee6f84674f2f34dceed894139562e29579ee7299e06756c8c990caddc5ed
  • Loading branch information
MarcoFalke authored and PastaPastaPasta committed Apr 7, 2022
1 parent e592537 commit 25f8e06
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/checkqueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,24 @@ class CCheckQueue
//! Add a batch of checks to the queue
void Add(std::vector<T>& vChecks)
{
LOCK(m_mutex);
for (T& check : vChecks) {
queue.push_back(T());
check.swap(queue.back());
if (vChecks.empty()) {
return;
}
nTodo += vChecks.size();
if (vChecks.size() == 1)

{
LOCK(m_mutex);
for (T& check : vChecks) {
queue.emplace_back();
check.swap(queue.back());
}
nTodo += vChecks.size();
}

if (vChecks.size() == 1) {
m_worker_cv.notify_one();
else if (vChecks.size() > 1)
} else {
m_worker_cv.notify_all();
}
}

//! Stop all of the worker threads.
Expand Down

0 comments on commit 25f8e06

Please sign in to comment.