Skip to content

Commit

Permalink
#Centipede Make PeriodicAction moveable by using PIMPL
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 655225949
  • Loading branch information
ussuri authored and copybara-github committed Jul 23, 2024
1 parent 17f8bba commit 42e98e0
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 78 deletions.
141 changes: 86 additions & 55 deletions centipede/periodic_action.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@

#include "./centipede/periodic_action.h"

#include <memory>
#include <thread>
#include <utility>

#include "absl/base/thread_annotations.h"
#include "absl/functional/any_invocable.h"
#include "absl/log/check.h"
#include "absl/synchronization/mutex.h"
Expand All @@ -24,65 +27,93 @@

namespace centipede {

PeriodicAction::PeriodicAction( //
absl::AnyInvocable<void()> action, Options options)
: action_{std::move(action)},
options_{std::move(options)},
thread_{[this]() { RunLoop(); }} {
// NOTE: Allow `options_.delay` to be `absl::InfiniteDuration()`: that's a
// valid use case, where the run-loop actually starts looping only after a
// first explicit nudge.
CHECK_GT(options_.interval, absl::ZeroDuration());
}

PeriodicAction::~PeriodicAction() { Stop(); }

void PeriodicAction::Stop() {
StopAsync();
// The run-loop should exit the next time it checks `stop_`. Note that if
// the loop is currently in the middle of an invocation of `action_`, it
// will wait for the invocation to finish, so we might block here for an
// `action_`-dependent amount of time.
if (thread_.joinable()) {
thread_.join();
class PeriodicAction::Impl {
public:
Impl(absl::AnyInvocable<void()> action, PeriodicAction::Options options)
: action_{std::move(action)},
options_{std::move(options)},
thread_{[this]() { RunLoop(); }} {
// NOTE: Allow `options_.delay` to be `absl::InfiniteDuration()`: that's a
// valid use case, where the run-loop actually starts looping only after a
// first explicit nudge.
CHECK_GT(options_.interval, absl::ZeroDuration());
}
}

void PeriodicAction::StopAsync() {
if (!stop_.HasBeenNotified()) {
// Prime the run-loop to exit next time it re-checks `stop_`.
stop_.Notify();
// Nudge the run-loop out of the sleeping phase, if it's there: the loop
// immediately goes to re-check `stop_` and exits.
{
absl::MutexLock lock{&nudge_mu_};
nudge_ = true;

void Stop() {
StopAsync();
// The run-loop should exit the next time it checks `stop_`. Note that if
// the loop is currently in the middle of an invocation of `action_`, it
// will wait for the invocation to finish, so we might block here for an
// `action_`-dependent amount of time.
if (thread_.joinable()) {
thread_.join();
}
}
}

void PeriodicAction::Nudge() {
absl::MutexLock lock{&nudge_mu_};
nudge_ = true;
}

void PeriodicAction::RunLoop() {
SleepUnlessWokenByNudge(options_.delay);
while (!stop_.HasBeenNotified()) {
action_();
SleepUnlessWokenByNudge(options_.interval);

void StopAsync() {
if (!stop_.HasBeenNotified()) {
// Prime the run-loop to exit next time it re-checks `stop_`.
stop_.Notify();
// Nudge the run-loop out of the sleeping phase, if it's there: the loop
// immediately goes to re-check `stop_` and exits.
{
absl::MutexLock lock{&nudge_mu_};
nudge_ = true;
}
}
}
}

void PeriodicAction::SleepUnlessWokenByNudge(absl::Duration duration) {
if (nudge_mu_.WriterLockWhenWithTimeout(absl::Condition{&nudge_}, duration)) {
// Got woken up by a nudge.
nudge_mu_.AssertHeld();
nudge_ = false;
} else {
// A nudge never came, slept well the entire time: nothing to do.

void Nudge() {
absl::MutexLock lock{&nudge_mu_};
nudge_ = true;
}

private:
void RunLoop() {
SleepUnlessWokenByNudge(options_.delay);
while (!stop_.HasBeenNotified()) {
action_();
SleepUnlessWokenByNudge(options_.interval);
}
}

void SleepUnlessWokenByNudge(absl::Duration duration) {
if (nudge_mu_.WriterLockWhenWithTimeout( //
absl::Condition{&nudge_}, duration)) {
// Got woken up by a nudge.
nudge_mu_.AssertHeld();
nudge_ = false;
} else {
// A nudge never came, slept well the entire time: nothing to do.
}
nudge_mu_.Unlock();
}
nudge_mu_.Unlock();
}

absl::AnyInvocable<void()> action_;
PeriodicAction::Options options_;

// WARNING!!! The order below is important.
absl::Notification stop_;
absl::Mutex nudge_mu_;
bool nudge_ ABSL_GUARDED_BY(nudge_mu_) = false;
std::thread thread_;
};

PeriodicAction::PeriodicAction( //
absl::AnyInvocable<void()> action, Options options)
: pimpl_{std::make_unique<Impl>(std::move(action), std::move(options))} {}

PeriodicAction::~PeriodicAction() { pimpl_->Stop(); }

void PeriodicAction::Stop() { pimpl_->Stop(); }

void PeriodicAction::StopAsync() { pimpl_->StopAsync(); }

void PeriodicAction::Nudge() { pimpl_->Nudge(); }

// NOTE: Even though these are defaulted, they still must be defined here in the
// .cc, because `Impl` is an incomplete type in the .h.
PeriodicAction::PeriodicAction(PeriodicAction&&) = default;
PeriodicAction& PeriodicAction::operator=(PeriodicAction&&) = default;

} // namespace centipede
26 changes: 8 additions & 18 deletions centipede/periodic_action.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,9 @@
#ifndef FUZZTEST_CENTIPEDE_PERIODIC_ACTION_H_
#define FUZZTEST_CENTIPEDE_PERIODIC_ACTION_H_

#include <thread>
#include <memory>

#include "absl/base/thread_annotations.h"
#include "absl/functional/any_invocable.h"
#include "absl/synchronization/mutex.h"
#include "absl/synchronization/notification.h"
#include "absl/time/time.h"

namespace centipede {
Expand All @@ -58,11 +55,9 @@ class PeriodicAction {

PeriodicAction(absl::AnyInvocable<void()> action, Options options);

// Non-copyable and non-movable.
PeriodicAction(const PeriodicAction&) = delete;
PeriodicAction& operator=(const PeriodicAction&) = delete;
PeriodicAction(PeriodicAction&&) = delete;
PeriodicAction& operator=(PeriodicAction&&) = delete;
// Movable, but not copyable.
PeriodicAction(PeriodicAction&&);
PeriodicAction& operator=(PeriodicAction&&);

// Stops the periodic action via RAII. May block: waits for any currently
// active invocation of the action to finish first before returning.
Expand Down Expand Up @@ -91,15 +86,10 @@ class PeriodicAction {
// which case wakes up and returns immediately.
void SleepUnlessWokenByNudge(absl::Duration duration);

absl::AnyInvocable<void()> action_;
const Options options_;

// WARNING!!! The order below is important.

absl::Notification stop_;
absl::Mutex nudge_mu_;
bool nudge_ ABSL_GUARDED_BY(nudge_mu_) = false;
std::thread thread_;
// Use the "pointer to implementation" idiom to make the class movable and
// move-constructible.
class Impl;
std::unique_ptr<Impl> pimpl_;
};

} // namespace centipede
Expand Down
9 changes: 4 additions & 5 deletions centipede/rusage_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -425,16 +425,15 @@ void RUsageProfiler::StartTimelapse( //
std::string title) {
absl::WriterMutexLock lock{&mutex_};
CHECK(!timelapse_recorder_) << "StopTimelapse() wasn't called";
const PeriodicAction::Options periodic_options{
.delay = absl::ZeroDuration(),
.interval = interval,
};
timelapse_recorder_ = std::make_unique<PeriodicAction>(
[this, loc = std::move(loc), title = std::move(title), also_log]() {
const auto& s = TakeSnapshot(loc, title);
if (also_log) s.Log();
},
periodic_options);
PeriodicAction::Options{
.delay = absl::ZeroDuration(),
.interval = interval,
});
}

void RUsageProfiler::StopTimelapse() {
Expand Down

0 comments on commit 42e98e0

Please sign in to comment.