From 543308fd9222a24617f8a9cc76f3ceb960935623 Mon Sep 17 00:00:00 2001 From: Sergei Shirokov Date: Thu, 22 Aug 2024 17:22:23 +0300 Subject: [PATCH 1/3] Added unit test for the "next_deadline" bug. --- tests/test_embedded_scheduler.cpp | 68 ++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/tests/test_embedded_scheduler.cpp b/tests/test_embedded_scheduler.cpp index 890bc4e..92ad589 100644 --- a/tests/test_embedded_scheduler.cpp +++ b/tests/test_embedded_scheduler.cpp @@ -24,12 +24,16 @@ #include #include #include +#include +#include +#include using testing::Gt; using testing::Le; using testing::Ne; using testing::IsNull; using testing::NotNull; +using testing::ElementsAre; // NOLINTBEGIN(bugprone-unchecked-optional-access) // NOLINTBEGIN(readability-function-cognitive-complexity, misc-const-correctness) @@ -90,6 +94,7 @@ TEST(TestEmbeddedScheduler, EventLoopBasic) // semihost::log(__LINE__, " ", out); EXPECT_THAT(out.next_deadline, SteadyClockMock::time_point::max()); EXPECT_THAT(out.worst_lateness, SteadyClockMock::duration::zero()); + EXPECT_THAT(out.approx_now.time_since_epoch(), 10'000ms); EXPECT_TRUE(evl->isEmpty()); EXPECT_THAT(evl->getTree()[0U], IsNull()); @@ -151,6 +156,7 @@ TEST(TestEmbeddedScheduler, EventLoopBasic) // semihost::log(__LINE__, " ", out); EXPECT_THAT(out.next_deadline.time_since_epoch(), 10'100ms); EXPECT_THAT(out.worst_lateness, 0ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 10'000ms); EXPECT_THAT(evl->getTree().size(), 4); EXPECT_FALSE(a); EXPECT_FALSE(b); @@ -159,11 +165,12 @@ TEST(TestEmbeddedScheduler, EventLoopBasic) // Make the first two expire. The one-shot two are still pending. SteadyClockMock::advance(1100ms); - EXPECT_THAT(11'100ms, SteadyClockMock::now().time_since_epoch()); + EXPECT_THAT(SteadyClockMock::now().time_since_epoch(), 11'100ms); out = evl->spin(); // semihost::log(__LINE__, " ", out); EXPECT_THAT(out.next_deadline.time_since_epoch(), 11'200ms); EXPECT_THAT(out.worst_lateness, 1000ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 11'100ms); EXPECT_THAT(evl->getTree().size(), 4); EXPECT_TRUE(a); EXPECT_THAT(a.value().deadline.time_since_epoch(), 11'000ms); @@ -182,6 +189,7 @@ TEST(TestEmbeddedScheduler, EventLoopBasic) // semihost::log(__LINE__, " ", out); EXPECT_THAT(out.next_deadline.time_since_epoch(), 12'100ms); EXPECT_THAT(out.worst_lateness, 800ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 12'000ms); EXPECT_THAT(evl->getTree()[0U]->getDeadline().value().time_since_epoch(), 12'100ms); EXPECT_THAT(evl->getTree().size(), 2); // C&D have left us. EXPECT_TRUE(a); @@ -214,12 +222,13 @@ TEST(TestEmbeddedScheduler, EventLoopBasic) EXPECT_THAT(evl->getTree().size(), 1); // Ditto. out = evl->spin(); // semihost::log(__LINE__, " ", out); - EXPECT_THAT(14'000ms, out.next_deadline.time_since_epoch()); // B removed so the next one is A. - EXPECT_THAT(50ms, out.worst_lateness); - EXPECT_THAT(14'000ms, evl->getTree()[0U]->getDeadline().value().time_since_epoch()); + EXPECT_THAT(out.next_deadline.time_since_epoch(), 14'000ms); // B removed so the next one is A. + EXPECT_THAT(out.worst_lateness, 50ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 13'050ms); + EXPECT_THAT(evl->getTree()[0U]->getDeadline().value().time_since_epoch(), 14'000ms); EXPECT_THAT(1, evl->getTree().size()); // Second dropped. EXPECT_TRUE(a); - EXPECT_THAT(13'000ms, a.value().deadline.time_since_epoch()); + EXPECT_THAT(a.value().deadline.time_since_epoch(), 13'000ms); EXPECT_FALSE(b); EXPECT_FALSE(c); EXPECT_FALSE(d); @@ -230,6 +239,7 @@ TEST(TestEmbeddedScheduler, EventLoopBasic) // semihost::log(__LINE__, " ", out); EXPECT_THAT(out.next_deadline.time_since_epoch(), 14'000ms); // Same up. EXPECT_THAT(out.worst_lateness, 0ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 13'050ms); EXPECT_FALSE(a); EXPECT_FALSE(b); EXPECT_FALSE(c); @@ -323,6 +333,54 @@ TEST(TestEmbeddedScheduler, EventLoopPoll) EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 210ms); // Skipped ahead! } +TEST(TestEmbeddedScheduler, EventLoopDefer_single_overdue) +{ + using time_point = SteadyClockMock::time_point; + using std::chrono_literals::operator""ms; + SteadyClockMock::reset(); + EventLoop evl; + + auto evt = evl.defer(SteadyClockMock::now() + 1000ms, [&](auto) {}); + EXPECT_TRUE(evt); + + // This is special case - only one deferred event (and no "repeat"-s!), and it is already overdue (by +30ms). + // So, `next_deadline` should be `time_point::max()` b/c there will be nothing left pending after spin. + SteadyClockMock::advance(1000ms + 30ms); + const auto out = evl.spin(); + EXPECT_THAT(out.next_deadline.time_since_epoch(), time_point::max().time_since_epoch()); + EXPECT_THAT(out.worst_lateness, 30ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 1030ms); +} + +TEST(TestEmbeddedScheduler, EventLoopDefer_long_running_callback) +{ + using duration = SteadyClockMock::duration; + using std::chrono_literals::operator""ms; + SteadyClockMock::reset(); + EventLoop evl; + + std::vector> calls; + + auto evt_a = evl.defer(SteadyClockMock::now() + 0ms, [&](const auto& arg) { // + // Emulate that it took whole 100ms to execute "a" callback, + // so it will be already overdue for the next "b" event - should be executed as well. + calls.emplace_back("a", arg.deadline.time_since_epoch(), arg.approx_now.time_since_epoch()); + SteadyClockMock::advance(100ms); + }); + auto evt_b = evl.defer(SteadyClockMock::now() + 20ms, [&](const auto& arg) { // + calls.emplace_back("b", arg.deadline.time_since_epoch(), arg.approx_now.time_since_epoch()); + }); + + const auto out = evl.spin(); + EXPECT_THAT(out.next_deadline.time_since_epoch(), duration::max()); + EXPECT_THAT(out.worst_lateness, 80ms); + EXPECT_THAT(out.approx_now.time_since_epoch(), 100ms); + + EXPECT_THAT(calls, + ElementsAre(std::make_tuple("a", 0ms, 0ms), // + std::make_tuple("b", 20ms, 100ms))); +} + TEST(TestEmbeddedScheduler, HandleMovement) { using std::chrono_literals::operator""ms; From c9c2fa8e470dc5ac1ca14ab424621f9756dc82dd Mon Sep 17 00:00:00 2001 From: Sergei Shirokov Date: Thu, 22 Aug 2024 17:39:28 +0300 Subject: [PATCH 2/3] Fix for the "next_deadline" bug. --- include/embedded_scheduler/scheduler.hpp | 35 +++++++++++++++--------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/include/embedded_scheduler/scheduler.hpp b/include/embedded_scheduler/scheduler.hpp index 8817230..ac872d3 100644 --- a/include/embedded_scheduler/scheduler.hpp +++ b/include/embedded_scheduler/scheduler.hpp @@ -301,32 +301,41 @@ class EventLoop final template [[nodiscard]] SpinResult spin() { - auto next = time_point::max(); - auto now = Clock::now(); - auto worst = duration::zero(); + SpinResult result{.next_deadline = time_point::max(), + .worst_lateness = duration::zero(), + .approx_now = time_point::min()}; + if (tree_.empty()) + { + result.approx_now = Clock::now(); + return result; + } + while (auto* const evt = static_cast(tree_.min())) { - next = evt->getDeadline().value(); // The deadline is guaranteed to be set because it is in the tree. - auto lateness = now - next; // Positive if late, zero if on-time. - if (lateness < duration::zero()) // Too early -- either we need to sleep or the time sample is obsolete. + // The deadline is guaranteed to be set because it is in the tree. + const auto deadline = evt->getDeadline().value(); + if (result.approx_now < deadline) // Too early -- either we need to sleep or the time sample is obsolete. { - now = Clock::now(); // The number of calls to Clock::now() is minimized. - lateness = now - next; - if (lateness < duration::zero()) // Nope, even with the latest time sample we are still early -- exit. + result.approx_now = Clock::now(); // The number of calls to Clock::now() is minimized. + if (result.approx_now < deadline) // Nope, even with the latest time sample we are still early -- exit. { + result.next_deadline = deadline; break; } } { ExecutionMonitor monitor{}; // RAII indication of the start and end of the event execution. // Execution will remove the event from the tree and then possibly re-insert it with a new deadline. - evt->execute({.event = *evt, .deadline = next, .approx_now = now}); + evt->execute({.event = *evt, .deadline = deadline, .approx_now = result.approx_now}); (void) monitor; } - worst = std::max(lateness, worst); + result.next_deadline = time_point::max(); // Reset the next deadline to the maximum possible value. + result.worst_lateness = std::max(result.worst_lateness, result.approx_now - deadline); } - assert(worst >= duration::zero()); - return {.next_deadline = next, .worst_lateness = worst, .approx_now = now}; + + assert(result.approx_now > time_point::min()); + assert(result.worst_lateness >= duration::zero()); + return result; } /// True if there are no registered events. From 83b2e2c40fd38c2031e041dc16baad23a5cad824 Mon Sep 17 00:00:00 2001 From: Sergei Shirokov Date: Fri, 23 Aug 2024 08:19:18 +0300 Subject: [PATCH 3/3] Fix for the "next_deadline" bug. --- TODO.md | 2 -- include/embedded_scheduler/scheduler.hpp | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/TODO.md b/TODO.md index c028e01..314158b 100644 --- a/TODO.md +++ b/TODO.md @@ -1,8 +1,6 @@ ## TODOs: - [ ] Search for... - - [ ] "dyshlo" - - [ ] "telega" - [ ] "serge" - [ ] Replace "serges147" with proper organization name when ready diff --git a/include/embedded_scheduler/scheduler.hpp b/include/embedded_scheduler/scheduler.hpp index ac872d3..41ed206 100644 --- a/include/embedded_scheduler/scheduler.hpp +++ b/include/embedded_scheduler/scheduler.hpp @@ -304,7 +304,7 @@ class EventLoop final SpinResult result{.next_deadline = time_point::max(), .worst_lateness = duration::zero(), .approx_now = time_point::min()}; - if (tree_.empty()) + if (tree_.empty()) [[unlikely]] { result.approx_now = Clock::now(); return result;