Skip to content

Commit

Permalink
Change uses of notify_all to notify_one in Hades
Browse files Browse the repository at this point in the history
Summary:
`std::condition_variable` has to use some extra bookkeeping to implement
`notify_all`, which is more costly than `notify_one`, even if there's only a single
other thread waiting on the variable.

Since Hades only has two threads, and only one of them is waiting on the variable at
a time, there's no need to notify more than a single thread.

Reviewed By: neildhar

Differential Revision: D23822972

fbshipit-source-id: 7ac5d2261b94e213407fb2293550f6b89c2d9df6
  • Loading branch information
Riley Dulin authored and facebook-github-bot committed Sep 21, 2020
1 parent c593863 commit 070ffdd
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/VM/gcs/HadesGC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ void HadesGC::waitForCollectionToFinish() {
worldStopped_ = true;
// Notify a waiting OG collection that it can complete.
lk.unlock();
stopTheWorldCondVar_.notify_all();
stopTheWorldCondVar_.notify_one();
// Wait for an existing collection to finish.
oldGenCollectionThread_.join();
// Undo the worldStopped_ change, as the mutator will now resume. Can't rely
Expand Down Expand Up @@ -1115,7 +1115,7 @@ void HadesGC::completeMarking() {
// Let the thread that yielded know that the STW pause has completed.
stopTheWorldRequested_ = false;
worldStopped_ = false;
stopTheWorldCondVar_.notify_all();
stopTheWorldCondVar_.notify_one();
}

void HadesGC::completeMarkingAssumeLocks() {
Expand Down Expand Up @@ -2337,7 +2337,7 @@ void HadesGC::yieldToOldGen() {
}
worldStopped_ = true;
// Notify a waiting OG collection that it can complete.
stopTheWorldCondVar_.notify_all();
stopTheWorldCondVar_.notify_one();
std::unique_lock<Mutex> lk{gcMutex_, std::adopt_lock};
waitForConditionVariable(
stopTheWorldCondVar_, lk, [this]() { return !worldStopped_; });
Expand Down

0 comments on commit 070ffdd

Please sign in to comment.