Improve ability to reschedule phased loops in AOS
This makes it so that users can reliably reschedule phased loops in a
well-defined way, for situations, e.g., where there may be a need to
update the offset of the loop dynamically.
This slightly changes the default semantics around dynamically changing
the interval/offset of a phased loop, but I don't actually see anyone
doing that currently, and there were some poorly-defined corner cases
anyways.
References: PRO-18058
Change-Id: I679c905da1582d250deb7f9607c8517ed34e5a25
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/event_loop.cc b/aos/events/event_loop.cc
index 6b5b601..f5e9f51 100644
--- a/aos/events/event_loop.cc
+++ b/aos/events/event_loop.cc
@@ -115,11 +115,7 @@
const monotonic_clock::time_point monotonic_now =
event_loop_->monotonic_now();
phased_loop_.Reset(monotonic_now);
- Reschedule(
- [this](monotonic_clock::time_point sleep_time) {
- Schedule(sleep_time);
- },
- monotonic_now);
+ Reschedule(monotonic_now);
// Reschedule here will count cycles elapsed before now, and then the
// reschedule before running the handler will count the time that elapsed
// then. So clear the count here.
diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index 471b625..23250e1 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -511,26 +511,61 @@
Ftrace ftrace_;
};
-// Interface for phased loops. They are built on timers.
+// Interface for phased loops. They are built on timers.
class PhasedLoopHandler {
public:
virtual ~PhasedLoopHandler();
- // Sets the interval and offset. Any changes to interval and offset only take
- // effect when the handler finishes running.
- void set_interval_and_offset(const monotonic_clock::duration interval,
- const monotonic_clock::duration offset) {
- phased_loop_.set_interval_and_offset(interval, offset);
+ // Sets the interval and offset. Any changes to interval and offset only take
+ // effect when the handler finishes running or upon a call to Reschedule().
+ //
+ // The precise semantics of the monotonic_now parameter are defined in
+ // phased_loop.h. The way that it gets used in this interface is to control
+ // what the cycles elapsed counter will read on the following call to your
+ // handler. In an idealized simulation environment, if you call
+ // set_interval_and_offset() during the phased loop offset without setting
+ // monotonic_now, then you should always see a count of 1 on the next cycle.
+ //
+ // With the default behavior (this is called inside your handler and with
+ // monotonic_now = nullopt), the next phased loop call will occur at most
+ // interval time after the current time. Note that in many cases this will
+ // *not* be the preferred behavior (in most cases, you would likely aim to
+ // keep the average frequency of the calls reasonably consistent).
+ //
+ // A combination of the monotonic_now parameter and manually calling
+ // Reschedule() outside of the phased loop handler can be used to alter the
+ // behavior of cycles_elapsed. For the default behavior, you can set
+ // monotonic_now to the current time. If you call set_interval_and_offset()
+ // and Reschedule() with the same monotonic_now, that will cause the next
+ // callback to occur in the range (monotonic_now, monotonic_now + interval]
+ // and get called with a count of 1 (if the event is actually able to happen
+ // when it is scheduled to). E.g., if you are just adjusting the phased loop
+ // offset (but not the interval) and want to maintain a consistent frequency,
+ // you may call these functions with monotonic_now = now + interval / 2.
+ void set_interval_and_offset(
+ const monotonic_clock::duration interval,
+ const monotonic_clock::duration offset,
+ std::optional<monotonic_clock::time_point> monotonic_now = std::nullopt) {
+ phased_loop_.set_interval_and_offset(interval, offset, monotonic_now);
}
- // Sets and gets the name of the timer. Set this if you want a descriptive
+ // Reruns the scheduler for the phased loop, scheduling the next callback at
+ // the next available time after monotonic_now. This allows
+ // set_interval_and_offset() to be called and have the changes take effect
+ // before the next handler finishes. This will have no effect if run during
+ // the phased loop's own handler.
+ void Reschedule(monotonic_clock::time_point monotonic_now) {
+ cycles_elapsed_ += phased_loop_.Iterate(monotonic_now);
+ Schedule(phased_loop_.sleep_time());
+ }
+
+ // Sets and gets the name of the timer. Set this if you want a descriptive
// name in the timing report.
void set_name(std::string_view name) { name_ = std::string(name); }
const std::string_view name() const { return name_; }
protected:
- void Call(std::function<monotonic_clock::time_point()> get_time,
- std::function<void(monotonic_clock::time_point)> schedule);
+ void Call(std::function<monotonic_clock::time_point()> get_time);
PhasedLoopHandler(EventLoop *event_loop, std::function<void(int)> fn,
const monotonic_clock::duration interval,
@@ -539,12 +574,6 @@
private:
friend class EventLoop;
- void Reschedule(std::function<void(monotonic_clock::time_point)> schedule,
- monotonic_clock::time_point monotonic_now) {
- cycles_elapsed_ += phased_loop_.Iterate(monotonic_now);
- schedule(phased_loop_.sleep_time());
- }
-
virtual void Schedule(monotonic_clock::time_point sleep_time) = 0;
EventLoop *event_loop_;
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index 0c8e41e..07bd6d4 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -1922,6 +1922,298 @@
EXPECT_GT(expected_times[expected_times.size() / 2], average_time - kEpsilon);
}
+// Tests that a phased loop responds correctly to a changing offset; sweep
+// across a variety of potential offset changes, to ensure that we are
+// exercising a variety of potential cases.
+TEST_P(AbstractEventLoopTest, PhasedLoopChangingOffsetSweep) {
+ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
+ const int kCount = 5;
+
+ auto loop1 = MakePrimary();
+
+ std::vector<aos::monotonic_clock::duration> offset_options;
+ for (int ii = 0; ii < kCount; ++ii) {
+ offset_options.push_back(ii * kInterval / kCount);
+ }
+ std::vector<aos::monotonic_clock::duration> offset_sweep;
+ // Run over all the pair-wise combinations of offsets.
+ for (int ii = 0; ii < kCount; ++ii) {
+ for (int jj = 0; jj < kCount; ++jj) {
+ offset_sweep.push_back(offset_options.at(ii));
+ offset_sweep.push_back(offset_options.at(jj));
+ }
+ }
+
+ std::vector<::aos::monotonic_clock::time_point> expected_times;
+
+ PhasedLoopHandler *phased_loop;
+
+ // Run kCount iterations.
+ size_t counter = 0;
+ phased_loop = loop1->AddPhasedLoop(
+ [&phased_loop, &expected_times, &loop1, this, kInterval, &counter,
+ offset_sweep](int count) {
+ EXPECT_EQ(count, 1);
+ expected_times.push_back(loop1->context().monotonic_event_time);
+
+ counter++;
+
+ if (counter == offset_sweep.size()) {
+ LOG(INFO) << "Exiting";
+ this->Exit();
+ return;
+ }
+
+ phased_loop->set_interval_and_offset(kInterval,
+ offset_sweep.at(counter));
+ },
+ kInterval, offset_sweep.at(0));
+
+ Run();
+ ASSERT_EQ(expected_times.size(), offset_sweep.size());
+ for (size_t ii = 1; ii < expected_times.size(); ++ii) {
+ EXPECT_LE(expected_times.at(ii) - expected_times.at(ii - 1), kInterval);
+ }
+}
+
+// Tests that a phased loop responds correctly to being rescheduled with now
+// equal to a time in the past.
+TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleInPast) {
+ const chrono::milliseconds kOffset = chrono::milliseconds(400);
+ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
+
+ auto loop1 = MakePrimary();
+
+ std::vector<::aos::monotonic_clock::time_point> expected_times;
+
+ PhasedLoopHandler *phased_loop;
+
+ int expected_count = 1;
+
+ // Set up a timer that will get run immediately after the phased loop and
+ // which will attempt to reschedule the phased loop to just before now. This
+ // should succeed, but will result in 0 cycles elapsing.
+ TimerHandler *manager_timer =
+ loop1->AddTimer([&phased_loop, &loop1, &expected_count, this]() {
+ if (expected_count == 0) {
+ LOG(INFO) << "Exiting";
+ this->Exit();
+ return;
+ }
+ phased_loop->Reschedule(loop1->context().monotonic_event_time -
+ std::chrono::nanoseconds(1));
+ expected_count = 0;
+ });
+
+ phased_loop = loop1->AddPhasedLoop(
+ [&expected_count, &expected_times, &loop1, manager_timer](int count) {
+ EXPECT_EQ(count, expected_count);
+ expected_times.push_back(loop1->context().monotonic_event_time);
+
+ manager_timer->Setup(loop1->context().monotonic_event_time);
+ },
+ kInterval, kOffset);
+ phased_loop->set_name("Test loop");
+ manager_timer->set_name("Manager timer");
+
+ Run();
+
+ ASSERT_EQ(2u, expected_times.size());
+ ASSERT_EQ(expected_times[0], expected_times[1]);
+}
+
+// Tests that a phased loop responds correctly to being rescheduled at the time
+// when it should be triggering (it should kick the trigger to the next cycle).
+TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleNow) {
+ const chrono::milliseconds kOffset = chrono::milliseconds(400);
+ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
+
+ auto loop1 = MakePrimary();
+
+ std::vector<::aos::monotonic_clock::time_point> expected_times;
+
+ PhasedLoopHandler *phased_loop;
+
+ bool should_exit = false;
+ // Set up a timer that will get run immediately after the phased loop and
+ // which will attempt to reschedule the phased loop to now. This should
+ // succeed, but will result in no change to the expected behavior (since this
+ // is the same thing that is actually done internally).
+ TimerHandler *manager_timer =
+ loop1->AddTimer([&phased_loop, &loop1, &should_exit, this]() {
+ if (should_exit) {
+ LOG(INFO) << "Exiting";
+ this->Exit();
+ return;
+ }
+ phased_loop->Reschedule(loop1->context().monotonic_event_time);
+ should_exit = true;
+ });
+
+ phased_loop = loop1->AddPhasedLoop(
+ [&expected_times, &loop1, manager_timer](int count) {
+ EXPECT_EQ(count, 1);
+ expected_times.push_back(loop1->context().monotonic_event_time);
+
+ manager_timer->Setup(loop1->context().monotonic_event_time);
+ },
+ kInterval, kOffset);
+ phased_loop->set_name("Test loop");
+ manager_timer->set_name("Manager timer");
+
+ Run();
+
+ ASSERT_EQ(2u, expected_times.size());
+ ASSERT_EQ(expected_times[0] + kInterval, expected_times[1]);
+}
+
+// Tests that a phased loop responds correctly to being rescheduled at a time in
+// the distant future.
+TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleFuture) {
+ const chrono::milliseconds kOffset = chrono::milliseconds(400);
+ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
+
+ auto loop1 = MakePrimary();
+
+ std::vector<::aos::monotonic_clock::time_point> expected_times;
+
+ PhasedLoopHandler *phased_loop;
+
+ bool should_exit = false;
+ int expected_count = 1;
+ TimerHandler *manager_timer = loop1->AddTimer(
+ [&expected_count, &phased_loop, &loop1, &should_exit, this, kInterval]() {
+ if (should_exit) {
+ LOG(INFO) << "Exiting";
+ this->Exit();
+ return;
+ }
+ expected_count = 10;
+ // Knock off 1 ns, since the scheduler rounds up when it is
+ // scheduled to exactly a loop time.
+ phased_loop->Reschedule(loop1->context().monotonic_event_time +
+ kInterval * expected_count -
+ std::chrono::nanoseconds(1));
+ should_exit = true;
+ });
+
+ phased_loop = loop1->AddPhasedLoop(
+ [&expected_times, &loop1, manager_timer, &expected_count](int count) {
+ EXPECT_EQ(count, expected_count);
+ expected_times.push_back(loop1->context().monotonic_event_time);
+
+ manager_timer->Setup(loop1->context().monotonic_event_time);
+ },
+ kInterval, kOffset);
+ phased_loop->set_name("Test loop");
+ manager_timer->set_name("Manager timer");
+
+ Run();
+
+ ASSERT_EQ(2u, expected_times.size());
+ ASSERT_EQ(expected_times[0] + expected_count * kInterval, expected_times[1]);
+}
+
+// Tests that a phased loop responds correctly to having its phase offset
+// incremented and then being scheduled after a set time, exercising a pattern
+// where a phased loop's offset is changed while trying to maintain the trigger
+// at a consistent period.
+TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleWithLaterOffset) {
+ const chrono::milliseconds kOffset = chrono::milliseconds(400);
+ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
+
+ auto loop1 = MakePrimary();
+
+ std::vector<::aos::monotonic_clock::time_point> expected_times;
+
+ PhasedLoopHandler *phased_loop;
+
+ bool should_exit = false;
+ TimerHandler *manager_timer = loop1->AddTimer(
+ [&phased_loop, &loop1, &should_exit, this, kInterval, kOffset]() {
+ if (should_exit) {
+ LOG(INFO) << "Exiting";
+ this->Exit();
+ return;
+ }
+ // Schedule the next callback to be strictly later than the current time
+ // + interval / 2, to ensure a consistent frequency.
+ monotonic_clock::time_point half_time =
+ loop1->context().monotonic_event_time + kInterval / 2;
+ phased_loop->set_interval_and_offset(
+ kInterval, kOffset + std::chrono::nanoseconds(1), half_time);
+ phased_loop->Reschedule(half_time);
+ should_exit = true;
+ });
+
+ phased_loop = loop1->AddPhasedLoop(
+ [&expected_times, &loop1, manager_timer](int count) {
+ EXPECT_EQ(1, count);
+ expected_times.push_back(loop1->context().monotonic_event_time);
+
+ manager_timer->Setup(loop1->context().monotonic_event_time);
+ },
+ kInterval, kOffset);
+ phased_loop->set_name("Test loop");
+ manager_timer->set_name("Manager timer");
+
+ Run();
+
+ ASSERT_EQ(2u, expected_times.size());
+ ASSERT_EQ(expected_times[0] + kInterval + std::chrono::nanoseconds(1),
+ expected_times[1]);
+}
+
+// Tests that a phased loop responds correctly to having its phase offset
+// decremented and then being scheduled after a set time, exercising a pattern
+// where a phased loop's offset is changed while trying to maintain the trigger
+// at a consistent period.
+TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleWithEarlierOffset) {
+ const chrono::milliseconds kOffset = chrono::milliseconds(400);
+ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
+
+ auto loop1 = MakePrimary();
+
+ std::vector<::aos::monotonic_clock::time_point> expected_times;
+
+ PhasedLoopHandler *phased_loop;
+
+ bool should_exit = false;
+ TimerHandler *manager_timer = loop1->AddTimer(
+ [&phased_loop, &loop1, &should_exit, this, kInterval, kOffset]() {
+ if (should_exit) {
+ LOG(INFO) << "Exiting";
+ this->Exit();
+ return;
+ }
+ // Schedule the next callback to be strictly later than the current time
+ // + interval / 2, to ensure a consistent frequency.
+ const aos::monotonic_clock::time_point half_time =
+ loop1->context().monotonic_event_time + kInterval / 2;
+ phased_loop->set_interval_and_offset(
+ kInterval, kOffset - std::chrono::nanoseconds(1), half_time);
+ phased_loop->Reschedule(half_time);
+ should_exit = true;
+ });
+
+ phased_loop = loop1->AddPhasedLoop(
+ [&expected_times, &loop1, manager_timer](int count) {
+ EXPECT_EQ(1, count);
+ expected_times.push_back(loop1->context().monotonic_event_time);
+
+ manager_timer->Setup(loop1->context().monotonic_event_time);
+ },
+ kInterval, kOffset);
+ phased_loop->set_name("Test loop");
+ manager_timer->set_name("Manager timer");
+
+ Run();
+
+ ASSERT_EQ(2u, expected_times.size());
+ ASSERT_EQ(expected_times[0] + kInterval - std::chrono::nanoseconds(1),
+ expected_times[1]);
+}
+
// Tests that senders count correctly in the timing report.
TEST_P(AbstractEventLoopTest, SenderTimingReport) {
FLAGS_timing_report_ms = 1000;
diff --git a/aos/events/event_loop_param_test.cc.rej b/aos/events/event_loop_param_test.cc.rej
new file mode 100644
index 0000000..c03b83d
--- /dev/null
+++ b/aos/events/event_loop_param_test.cc.rej
@@ -0,0 +1,300 @@
+diff a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc (rejected hunks)
+@@ -1923,6 +1923,298 @@
+ EXPECT_GT(expected_times[expected_times.size() / 2], average_time - kEpsilon);
+ }
+
++// Tests that a phased loop responds correctly to a changing offset; sweep
++// across a variety of potential offset changes, to ensure that we are
++// exercising a variety of potential cases.
++TEST_P(AbstractEventLoopTest, PhasedLoopChangingOffsetSweep) {
++ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
++ const int kCount = 5;
++
++ auto loop1 = MakePrimary();
++
++ std::vector<aos::monotonic_clock::duration> offset_options;
++ for (int ii = 0; ii < kCount; ++ii) {
++ offset_options.push_back(ii * kInterval / kCount);
++ }
++ std::vector<aos::monotonic_clock::duration> offset_sweep;
++ // Run over all the pair-wise combinations of offsets.
++ for (int ii = 0; ii < kCount; ++ii) {
++ for (int jj = 0; jj < kCount; ++jj) {
++ offset_sweep.push_back(offset_options.at(ii));
++ offset_sweep.push_back(offset_options.at(jj));
++ }
++ }
++
++ std::vector<::aos::monotonic_clock::time_point> expected_times;
++
++ PhasedLoopHandler *phased_loop;
++
++ // Run kCount iterations.
++ int counter = 0;
++ phased_loop = loop1->AddPhasedLoop(
++ [&phased_loop, &expected_times, &loop1, this, kInterval, &counter,
++ offset_sweep](int count) {
++ EXPECT_EQ(count, 1);
++ expected_times.push_back(loop1->context().monotonic_event_time);
++
++ counter++;
++
++ if (counter == offset_sweep.size()) {
++ LOG(INFO) << "Exiting";
++ this->Exit();
++ return;
++ }
++
++ phased_loop->set_interval_and_offset(kInterval,
++ offset_sweep.at(counter));
++ },
++ kInterval, offset_sweep.at(0));
++
++ Run();
++ ASSERT_EQ(expected_times.size(), offset_sweep.size());
++ for (size_t ii = 1; ii < expected_times.size(); ++ii) {
++ EXPECT_LE(expected_times.at(ii) - expected_times.at(ii - 1), kInterval);
++ }
++}
++
++// Tests that a phased loop responds correctly to being rescheduled with now
++// equal to a time in the past.
++TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleInPast) {
++ const chrono::milliseconds kOffset = chrono::milliseconds(400);
++ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
++
++ auto loop1 = MakePrimary();
++
++ std::vector<::aos::monotonic_clock::time_point> expected_times;
++
++ PhasedLoopHandler *phased_loop;
++
++ int expected_count = 1;
++
++ // Set up a timer that will get run immediately after the phased loop and
++ // which will attempt to reschedule the phased loop to just before now. This
++ // should succeed, but will result in 0 cycles elapsing.
++ TimerHandler *manager_timer =
++ loop1->AddTimer([&phased_loop, &loop1, &expected_count, this]() {
++ if (expected_count == 0) {
++ LOG(INFO) << "Exiting";
++ this->Exit();
++ return;
++ }
++ phased_loop->Reschedule(loop1->context().monotonic_event_time -
++ std::chrono::nanoseconds(1));
++ expected_count = 0;
++ });
++
++ phased_loop = loop1->AddPhasedLoop(
++ [&expected_count, &expected_times, &loop1, manager_timer](int count) {
++ EXPECT_EQ(count, expected_count);
++ expected_times.push_back(loop1->context().monotonic_event_time);
++
++ manager_timer->Setup(loop1->context().monotonic_event_time);
++ },
++ kInterval, kOffset);
++ phased_loop->set_name("Test loop");
++ manager_timer->set_name("Manager timer");
++
++ Run();
++
++ ASSERT_EQ(2u, expected_times.size());
++ ASSERT_EQ(expected_times[0], expected_times[1]);
++}
++
++// Tests that a phased loop responds correctly to being rescheduled at the time
++// when it should be triggering (it should kick the trigger to the next cycle).
++TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleNow) {
++ const chrono::milliseconds kOffset = chrono::milliseconds(400);
++ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
++
++ auto loop1 = MakePrimary();
++
++ std::vector<::aos::monotonic_clock::time_point> expected_times;
++
++ PhasedLoopHandler *phased_loop;
++
++ bool should_exit = false;
++ // Set up a timer that will get run immediately after the phased loop and
++ // which will attempt to reschedule the phased loop to now. This should
++ // succeed, but will result in no change to the expected behavior (since this
++ // is the same thing that is actually done internally).
++ TimerHandler *manager_timer =
++ loop1->AddTimer([&phased_loop, &loop1, &should_exit, this]() {
++ if (should_exit) {
++ LOG(INFO) << "Exiting";
++ this->Exit();
++ return;
++ }
++ phased_loop->Reschedule(loop1->context().monotonic_event_time);
++ should_exit = true;
++ });
++
++ phased_loop = loop1->AddPhasedLoop(
++ [&expected_times, &loop1, manager_timer](int count) {
++ EXPECT_EQ(count, 1);
++ expected_times.push_back(loop1->context().monotonic_event_time);
++
++ manager_timer->Setup(loop1->context().monotonic_event_time);
++ },
++ kInterval, kOffset);
++ phased_loop->set_name("Test loop");
++ manager_timer->set_name("Manager timer");
++
++ Run();
++
++ ASSERT_EQ(2u, expected_times.size());
++ ASSERT_EQ(expected_times[0] + kInterval, expected_times[1]);
++}
++
++// Tests that a phased loop responds correctly to being rescheduled at a time in
++// the distant future.
++TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleFuture) {
++ const chrono::milliseconds kOffset = chrono::milliseconds(400);
++ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
++
++ auto loop1 = MakePrimary();
++
++ std::vector<::aos::monotonic_clock::time_point> expected_times;
++
++ PhasedLoopHandler *phased_loop;
++
++ bool should_exit = false;
++ int expected_count = 1;
++ TimerHandler *manager_timer = loop1->AddTimer(
++ [&expected_count, &phased_loop, &loop1, &should_exit, this, kInterval]() {
++ if (should_exit) {
++ LOG(INFO) << "Exiting";
++ this->Exit();
++ return;
++ }
++ expected_count = 10;
++ // Knock off 1 ns, since the scheduler rounds up when it is
++ // scheduled to exactly a loop time.
++ phased_loop->Reschedule(loop1->context().monotonic_event_time +
++ kInterval * expected_count -
++ std::chrono::nanoseconds(1));
++ should_exit = true;
++ });
++
++ phased_loop = loop1->AddPhasedLoop(
++ [&expected_times, &loop1, manager_timer, &expected_count](int count) {
++ EXPECT_EQ(count, expected_count);
++ expected_times.push_back(loop1->context().monotonic_event_time);
++
++ manager_timer->Setup(loop1->context().monotonic_event_time);
++ },
++ kInterval, kOffset);
++ phased_loop->set_name("Test loop");
++ manager_timer->set_name("Manager timer");
++
++ Run();
++
++ ASSERT_EQ(2u, expected_times.size());
++ ASSERT_EQ(expected_times[0] + expected_count * kInterval, expected_times[1]);
++}
++
++// Tests that a phased loop responds correctly to having its phase offset
++// incremented and then being scheduled after a set time, exercising a pattern
++// where a phased loop's offset is changed while trying to maintain the trigger
++// at a consistent period.
++TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleWithLaterOffset) {
++ const chrono::milliseconds kOffset = chrono::milliseconds(400);
++ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
++
++ auto loop1 = MakePrimary();
++
++ std::vector<::aos::monotonic_clock::time_point> expected_times;
++
++ PhasedLoopHandler *phased_loop;
++
++ bool should_exit = false;
++ TimerHandler *manager_timer = loop1->AddTimer(
++ [&phased_loop, &loop1, &should_exit, this, kInterval, kOffset]() {
++ if (should_exit) {
++ LOG(INFO) << "Exiting";
++ this->Exit();
++ return;
++ }
++ // Schedule the next callback to be strictly later than the current time
++ // + interval / 2, to ensure a consistent frequency.
++ monotonic_clock::time_point half_time =
++ loop1->context().monotonic_event_time + kInterval / 2;
++ phased_loop->set_interval_and_offset(
++ kInterval, kOffset + std::chrono::nanoseconds(1), half_time);
++ phased_loop->Reschedule(half_time);
++ should_exit = true;
++ });
++
++ phased_loop = loop1->AddPhasedLoop(
++ [&expected_times, &loop1, manager_timer](int count) {
++ EXPECT_EQ(1, count);
++ expected_times.push_back(loop1->context().monotonic_event_time);
++
++ manager_timer->Setup(loop1->context().monotonic_event_time);
++ },
++ kInterval, kOffset);
++ phased_loop->set_name("Test loop");
++ manager_timer->set_name("Manager timer");
++
++ Run();
++
++ ASSERT_EQ(2u, expected_times.size());
++ ASSERT_EQ(expected_times[0] + kInterval + std::chrono::nanoseconds(1),
++ expected_times[1]);
++}
++
++// Tests that a phased loop responds correctly to having its phase offset
++// decremented and then being scheduled after a set time, exercising a pattern
++// where a phased loop's offset is changed while trying to maintain the trigger
++// at a consistent period.
++TEST_P(AbstractEventLoopTest, PhasedLoopRescheduleWithEarlierOffset) {
++ const chrono::milliseconds kOffset = chrono::milliseconds(400);
++ const chrono::milliseconds kInterval = chrono::milliseconds(1000);
++
++ auto loop1 = MakePrimary();
++
++ std::vector<::aos::monotonic_clock::time_point> expected_times;
++
++ PhasedLoopHandler *phased_loop;
++
++ bool should_exit = false;
++ TimerHandler *manager_timer = loop1->AddTimer(
++ [&phased_loop, &loop1, &should_exit, this, kInterval, kOffset]() {
++ if (should_exit) {
++ LOG(INFO) << "Exiting";
++ this->Exit();
++ return;
++ }
++ // Schedule the next callback to be strictly later than the current time
++ // + interval / 2, to ensure a consistent frequency.
++ const aos::monotonic_clock::time_point half_time =
++ loop1->context().monotonic_event_time + kInterval / 2;
++ phased_loop->set_interval_and_offset(
++ kInterval, kOffset - std::chrono::nanoseconds(1), half_time);
++ phased_loop->Reschedule(half_time);
++ should_exit = true;
++ });
++
++ phased_loop = loop1->AddPhasedLoop(
++ [&expected_times, &loop1, manager_timer](int count) {
++ EXPECT_EQ(1, count);
++ expected_times.push_back(loop1->context().monotonic_event_time);
++
++ manager_timer->Setup(loop1->context().monotonic_event_time);
++ },
++ kInterval, kOffset);
++ phased_loop->set_name("Test loop");
++ manager_timer->set_name("Manager timer");
++
++ Run();
++
++ ASSERT_EQ(2u, expected_times.size());
++ ASSERT_EQ(expected_times[0] + kInterval - std::chrono::nanoseconds(1),
++ expected_times[1]);
++}
++
+ // Tests that senders count correctly in the timing report.
+ TEST_P(AbstractEventLoopTest, SenderTimingReport) {
+ gflags::FlagSaver flag_saver;
diff --git a/aos/events/event_loop_tmpl.h b/aos/events/event_loop_tmpl.h
index d206bc1..39ab43e 100644
--- a/aos/events/event_loop_tmpl.h
+++ b/aos/events/event_loop_tmpl.h
@@ -238,8 +238,7 @@
}
inline void PhasedLoopHandler::Call(
- std::function<monotonic_clock::time_point()> get_time,
- std::function<void(monotonic_clock::time_point)> schedule) {
+ std::function<monotonic_clock::time_point()> get_time) {
// Read time directly to save a vtable indirection...
const monotonic_clock::time_point monotonic_start_time = get_time();
@@ -270,7 +269,7 @@
cycles_elapsed_ = 0;
// Schedule the next wakeup.
- schedule(phased_loop_.sleep_time());
+ Schedule(phased_loop_.sleep_time());
const monotonic_clock::time_point monotonic_end_time = get_time();
ftrace_.FormatMessage(
@@ -287,7 +286,7 @@
// If the handler took too long so we blew by the previous deadline, we
// want to just try for the next deadline. Reschedule.
if (monotonic_end_time > phased_loop_.sleep_time()) {
- Reschedule(schedule, monotonic_end_time);
+ Reschedule(monotonic_end_time);
}
}
diff --git a/aos/events/shm_event_loop.cc b/aos/events/shm_event_loop.cc
index 56d2357..89d0a9c 100644
--- a/aos/events/shm_event_loop.cc
+++ b/aos/events/shm_event_loop.cc
@@ -679,9 +679,7 @@
timerfd_.Read();
event_.Invalidate();
- Call(monotonic_clock::now, [this](monotonic_clock::time_point sleep_time) {
- Schedule(sleep_time);
- });
+ Call(monotonic_clock::now);
}
~ShmPhasedLoopHandler() override {
diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index 22b9603..c679b21 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -1293,10 +1293,7 @@
{
ScopedMarkRealtimeRestorer rt(
simulated_event_loop_->runtime_realtime_priority() > 0);
- Call([monotonic_now]() { return monotonic_now; },
- [this](monotonic_clock::time_point sleep_time) {
- Schedule(sleep_time);
- });
+ Call([monotonic_now]() { return monotonic_now; });
simulated_event_loop_->ClearContext();
}
}
@@ -1312,6 +1309,7 @@
// The allocations in here are due to infrastructure and don't count in the no
// mallocs in RT code.
ScopedNotRealtime nrt;
+ simulated_event_loop_->RemoveEvent(&event_);
if (token_ != scheduler_->InvalidToken()) {
scheduler_->Deschedule(token_);
token_ = scheduler_->InvalidToken();
diff --git a/aos/util/phased_loop.cc b/aos/util/phased_loop.cc
index 53f6f4d..1f61775 100644
--- a/aos/util/phased_loop.cc
+++ b/aos/util/phased_loop.cc
@@ -17,15 +17,26 @@
void PhasedLoop::set_interval_and_offset(
const monotonic_clock::duration interval,
- const monotonic_clock::duration offset) {
+ const monotonic_clock::duration offset,
+ std::optional<monotonic_clock::time_point> monotonic_now) {
// Update last_time_ to the new offset so that we have an even interval
- last_time_ += offset - offset_;
+ // In doing so, set things so that last_time_ will only ever decrease on calls
+ // to set_interval_and_offset.
+ last_time_ += offset - offset_ -
+ (offset > offset_ ? interval : monotonic_clock::duration(0));
interval_ = interval;
offset_ = offset;
CHECK(offset_ >= monotonic_clock::duration(0));
CHECK(interval_ > monotonic_clock::duration(0));
CHECK(offset_ < interval_);
+ // Reset effectively clears the skipped iteration count and ensures that the
+ // last time is in the interval (monotonic_now - interval, monotonic_now],
+ // which means that a call to Iterate(monotonic_now) will return 1 and set a
+ // wakeup time after monotonic_now.
+ if (monotonic_now.has_value()) {
+ Iterate(monotonic_now.value());
+ }
}
monotonic_clock::duration PhasedLoop::OffsetFromIntervalAndTime(
diff --git a/aos/util/phased_loop.h b/aos/util/phased_loop.h
index 307b400..64a3302 100644
--- a/aos/util/phased_loop.h
+++ b/aos/util/phased_loop.h
@@ -1,6 +1,8 @@
#ifndef AOS_UTIL_PHASED_LOOP_H_
#define AOS_UTIL_PHASED_LOOP_H_
+#include <optional>
+
#include "aos/time/time.h"
namespace aos {
@@ -21,8 +23,30 @@
const monotonic_clock::duration offset = monotonic_clock::duration(0));
// Updates the offset and interval.
- void set_interval_and_offset(const monotonic_clock::duration interval,
- const monotonic_clock::duration offset);
+ //
+ // After a call to set_interval_and_offset with monotonic_now = nullopt, the
+ // following will hold, for any allowed values of interval and offset:
+ // auto original_time = loop.sleep_time();
+ // loop.set_interval_and_offset(interval, offset);
+ // CHECK_LE(loop.sleep_time(), original_time);
+ // CHECK_EQ(0, loop.Iterate(original_time));
+ //
+ // Note that this will not be the behavior that all (or even necessarily most)
+ // users want, since it doesn't necessarily preserve a "keep the iteration
+ // time as consistent as possible" concept. However, it *is* better defined
+ // than the alternative, where if you decrease the offset by, e.g., 1ms on a
+ // 100ms interval, then the behavior will vary depending on whather you are
+ // going from 0ms->999ms offset or from 1ms->0ms offset.
+ //
+ // If monotonic_now is set, then the following will hold:
+ // auto original_time = loop.sleep_time();
+ // loop.set_interval_and_offset(interval, offset, monotonic_now);
+ // CHECK_LE(loop.sleep_time(), monotonic_now);
+ // CHECK_EQ(0, loop.Iterate(monotonic_now));
+ void set_interval_and_offset(
+ const monotonic_clock::duration interval,
+ const monotonic_clock::duration offset,
+ std::optional<monotonic_clock::time_point> monotonic_now = std::nullopt);
// Computes the offset given an interval and a time that we should trigger.
static monotonic_clock::duration OffsetFromIntervalAndTime(
diff --git a/aos/util/phased_loop_test.cc b/aos/util/phased_loop_test.cc
index 5e85ea0..de4483c 100644
--- a/aos/util/phased_loop_test.cc
+++ b/aos/util/phased_loop_test.cc
@@ -1,6 +1,7 @@
#include "aos/util/phased_loop.h"
#include "aos/time/time.h"
+#include "glog/logging.h"
#include "gtest/gtest.h"
namespace aos {
@@ -230,7 +231,23 @@
ASSERT_EQ(5, loop.Iterate(last_time));
for (int i = 1; i < kCount; i++) {
const auto offset = kOffset - milliseconds(i);
- loop.set_interval_and_offset(kInterval, offset);
+ // First, set the interval/offset without specifying a "now". If we then
+ // attempt to Iterate() to the same time as the last iteration, this should
+ // always result in zero cycles elapsed.
+ {
+ const monotonic_clock::time_point original_time = loop.sleep_time();
+ loop.set_interval_and_offset(kInterval, offset);
+ EXPECT_EQ(original_time - milliseconds(1), loop.sleep_time());
+ EXPECT_EQ(0, loop.Iterate(last_time));
+ }
+
+ // Now, explicitly update/clear things to last_time. This should have the
+ // same behavior as not specifying a monotonic_now.
+ {
+ loop.set_interval_and_offset(kInterval, offset, last_time);
+ EXPECT_EQ(0, loop.Iterate(last_time));
+ }
+
const auto next_time = last_time - milliseconds(1) + kAllIterationsInterval;
EXPECT_EQ(kIterations, loop.Iterate(next_time));
last_time = next_time;
@@ -238,6 +255,43 @@
}
// Tests that the phased loop is correctly adjusting when the offset is
+// incremented multiple times.
+TEST_F(PhasedLoopTest, IncrementingOffset) {
+ constexpr int kCount = 5;
+ constexpr int kIterations = 10;
+ const auto kOffset = milliseconds(0);
+ const auto kInterval = milliseconds(1000);
+ const auto kAllIterationsInterval = kInterval * kIterations;
+
+ PhasedLoop loop(kInterval, monotonic_clock::epoch(), kOffset);
+ auto last_time = monotonic_clock::epoch() + kOffset + (kInterval * 3);
+ ASSERT_EQ(4, loop.Iterate(last_time));
+ for (int i = 1; i < kCount; i++) {
+ const auto offset = kOffset + milliseconds(i);
+ {
+ const monotonic_clock::time_point original_time = loop.sleep_time();
+ loop.set_interval_and_offset(kInterval, offset);
+ EXPECT_EQ(original_time - kInterval + milliseconds(1), loop.sleep_time());
+ EXPECT_EQ(0, loop.Iterate(last_time));
+ }
+ // Now, explicitly update/clear things to a set time. We add a milliseconds
+ // so that when we call Iterate() next we actually get the expected number
+ // of iterations (otherwise, there is an iteration that would happen at
+ // last_time + 1 that gets counted, which is correct behavior, and so just
+ // needs to be accounted for somehow).
+ {
+ loop.set_interval_and_offset(kInterval, offset,
+ last_time + milliseconds(1));
+ EXPECT_EQ(0, loop.Iterate(last_time + milliseconds(1)));
+ }
+
+ const auto next_time = last_time + milliseconds(1) + kAllIterationsInterval;
+ EXPECT_EQ(kIterations, loop.Iterate(next_time));
+ last_time = next_time;
+ }
+}
+
+// Tests that the phased loop is correctly adjusting when the offset is
// changed to 0.
TEST_F(PhasedLoopTest, ChangingOffset) {
const auto kOffset = milliseconds(900);