Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conserve 8 bytes on Event #12

Merged
merged 4 commits into from
Dec 4, 2024
Merged

Conserve 8 bytes on Event #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8fe9ed
opening MR
maksimdrachov Dec 4, 2024
29ffc4c
decrease memory usage event
maksimdrachov Dec 4, 2024
6bcb164
clang-tidy
maksimdrachov Dec 4, 2024
0bd1a33
Sergei comments
maksimdrachov Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions include/olga_scheduler/olga_scheduler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,21 @@ class Event : private cavl::Node<Event<TimePoint>>
void cancel() noexcept
{
/// It is guaranteed that while an event resides in the tree, it has a valid deadline set.
if (deadline_)
if (deadline_ != TimePoint::min())
{
// Removing a non-existent node from the tree is an UB that may lead to memory corruption,
// so we have to check first if the event is actually registered.
remove();
// This is used to mark the event as unregistered so we don't double-remove it.
// This can only be done after the event is removed from the tree.
deadline_.reset();
deadline_ = TimePoint::min();
}
}

/// Diagnostic accessor for testing. Not intended for normal use.
/// Empty option means that the event is not scheduled or canceled.
/// The deadline is TimePoint::min() if the event is not scheduled or canceled.
/// It is guaranteed that while an event resides in the tree, it has a valid deadline set.
[[nodiscard]] std::optional<TimePoint> getDeadline() const noexcept { return deadline_; }
[[nodiscard]] TimePoint getDeadline() const noexcept { return deadline_; }
maksimdrachov marked this conversation as resolved.
Show resolved Hide resolved

// This method is necessary to store an Event in cetl::unbounded_variant
static constexpr std::array<std::uint8_t, 16> _get_type_id_() noexcept
Expand All @@ -115,14 +115,14 @@ class Event : private cavl::Node<Event<TimePoint>>
virtual ~Event()
{
cancel();
assert(!deadline_.has_value());
assert(deadline_ == TimePoint::min());
assert((this->getChildNode(false) == nullptr) && (this->getChildNode(true) == nullptr) &&
(this->getParentNode() == nullptr));
}

Event(Event&& other) noexcept :
cavl::Node<Event>{std::move(static_cast<cavl::Node<Event>&>(other))},
deadline_{std::exchange(other.deadline_, std::nullopt)}
deadline_{std::exchange(other.deadline_, TimePoint::min())}
{}

/// Ensure the event is in the tree and set the deadline to the specified absolute time point.
Expand All @@ -136,7 +136,7 @@ class Event : private cavl::Node<Event<TimePoint>>
[dead](const Event& other) {
/// No two deadlines compare equal, which allows us to have multiple nodes with the same deadline in
/// the tree. With two nodes sharing the same deadline, the one added later is considered to be later.
return (dead >= other.deadline_.value()) ? +1 : -1;
return (dead >= other.deadline_) ? +1 : -1;
},
[this] { return this; });
assert(std::get<0>(ptr_existing) == this);
Expand All @@ -151,7 +151,7 @@ class Event : private cavl::Node<Event<TimePoint>>
virtual void execute(const Arg<TimePoint>& args, Tree& tree) = 0;

private:
std::optional<TimePoint> deadline_;
TimePoint deadline_ = TimePoint::min();
};

/// This information is returned by the spin() method to allow the caller to decide what to do next
Expand Down Expand Up @@ -323,7 +323,7 @@ class EventLoop final
while (auto* const evt = static_cast<EventProxy*>(tree_.min()))
{
// The deadline is guaranteed to be set because it is in the tree.
const auto deadline = evt->getDeadline().value();
const auto deadline = evt->getDeadline();
if (result.approx_now < deadline) // Too early -- either we need to sleep or the time sample is obsolete.
{
result.approx_now = Clock::now(); // The number of calls to Clock::now() is minimized.
Expand Down
61 changes: 29 additions & 32 deletions tests/test_olga_scheduler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace
{

/// Expected type ID for the type returned from repeat(), poll() and defer().
constexpr std::array<std::uint8_t, 16> expected_type_id =
constexpr std::array<std::uint8_t, 16> event_type_id =
{0xB6, 0x87, 0x48, 0xA6, 0x7A, 0xDB, 0x4D, 0xF1, 0xB3, 0x1D, 0xA9, 0x8D, 0x50, 0xA7, 0x82, 0x47};

/// This clock has to keep global state to implement the TrivialClock trait.
Expand Down Expand Up @@ -104,37 +104,36 @@ TEST(TestOlgaScheduler, EventLoopBasic)

// Register our handlers. Events with same deadline are ordered such that the one added later is processed later.
auto evt_a = evl.repeat(1000ms, [&](const auto& arg) { a.emplace(arg); });
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[1U], IsNull());
EXPECT_FALSE(evl.isEmpty());

// Check the type ID of return type repeat().
const auto actual_type_id = decltype(evt_a)::_get_type_id_();
EXPECT_THAT(actual_type_id, ElementsAreArray(expected_type_id));
EXPECT_THAT(decltype(evt_a)::_get_type_id_(), ElementsAreArray(event_type_id));

auto evt_b = evl.repeat(100ms, // Smaller deadline goes on the left.
[&](const auto& arg) { b.emplace(arg); });
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 10'100ms);
EXPECT_THAT(evl.getTree()[1U]->getDeadline().value().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 10'100ms);
EXPECT_THAT(evl.getTree()[1U]->getDeadline().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[2U], IsNull());

auto evt_c = evl.defer(SteadyClockMock::now() + 2000ms, [&](const auto& arg) { c.emplace(arg); });
// EXPECT_THAT(evt_c, Optional(_));
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 10'100ms);
EXPECT_THAT(evl.getTree()[1U]->getDeadline().value().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 10'100ms);
EXPECT_THAT(evl.getTree()[1U]->getDeadline().time_since_epoch(), 11'000ms);
const auto* const f3 = evl.getTree()[2U];
EXPECT_THAT(f3->getDeadline().value().time_since_epoch(), 12'000ms); // New entry.
EXPECT_THAT(f3->getDeadline().time_since_epoch(), 12'000ms); // New entry.
EXPECT_THAT(evl.getTree()[3U], IsNull());

auto evt_d = evl.defer(SteadyClockMock::now() + 2000ms, // Same deadline!
[&](const auto& arg) { d.emplace(arg); });
// EXPECT_THAT(evt_d, NotNull());
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 10'100ms);
EXPECT_THAT(evl.getTree()[1U]->getDeadline().value().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 10'100ms);
EXPECT_THAT(evl.getTree()[1U]->getDeadline().time_since_epoch(), 11'000ms);
EXPECT_THAT(evl.getTree()[2U], f3); // Entry added before this one.
const auto* const f4 = evl.getTree()[3U];
EXPECT_THAT(f3, Ne(f4));
EXPECT_THAT(f4->getDeadline().value().time_since_epoch(), 12'000ms); // New entry, same deadline added later.
EXPECT_THAT(f4->getDeadline().time_since_epoch(), 12'000ms); // New entry, same deadline added later.
EXPECT_THAT(evl.getTree()[4U], IsNull());

// Poll but there are no pending Events yet.
Expand Down Expand Up @@ -164,7 +163,7 @@ TEST(TestOlgaScheduler, EventLoopBasic)
EXPECT_FALSE(d);
a.reset();
b.reset();
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 11'200ms);
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 11'200ms);

// Move on. Let C&D fire, they are canceled automatically.
SteadyClockMock::advance(900ms);
Expand All @@ -173,7 +172,7 @@ TEST(TestOlgaScheduler, EventLoopBasic)
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()[0U]->getDeadline().time_since_epoch(), 12'100ms);
EXPECT_THAT(evl.getTree().size(), 2); // C&D have left us.
EXPECT_TRUE(a);
EXPECT_THAT(a.value().deadline.time_since_epoch(), 12'000ms);
Expand All @@ -188,26 +187,26 @@ TEST(TestOlgaScheduler, EventLoopBasic)
c.reset();
d.reset();
// Ensure the deadline is cleared on those events that are canceled.
EXPECT_TRUE(evt_a.getDeadline());
EXPECT_TRUE(evt_b.getDeadline());
EXPECT_FALSE(evt_c.getDeadline());
EXPECT_FALSE(evt_d.getDeadline());
EXPECT_THAT(evt_a.getDeadline(), Gt(Loop::time_point::min()));
EXPECT_THAT(evt_b.getDeadline(), Gt(Loop::time_point::min()));
EXPECT_THAT(evt_c.getDeadline(), Loop::time_point::min());
EXPECT_THAT(evt_d.getDeadline(), Loop::time_point::min());

// Drop the second event and ensure it is removed from the tree immediately.
SteadyClockMock::advance(1050ms);
EXPECT_THAT(SteadyClockMock::now().time_since_epoch(), 13'050ms);
EXPECT_TRUE(evt_b.getDeadline());
EXPECT_THAT(evt_b.getDeadline(), Gt(Loop::time_point::min()));
evt_b.cancel();
EXPECT_FALSE(evt_b.getDeadline()); // Unregistered, cleared.
EXPECT_THAT(evl.getTree().size(), 1); // Freed already.
evt_b.cancel(); // Idempotency.
EXPECT_FALSE(evt_b.getDeadline()); // Ditto.
EXPECT_THAT(evl.getTree().size(), 1); // Ditto.
EXPECT_THAT(evt_b.getDeadline(), Loop::time_point::min()); // Unregistered, cleared.
EXPECT_THAT(evl.getTree().size(), 1); // Freed already.
evt_b.cancel(); // Idempotency.
EXPECT_THAT(evt_b.getDeadline(), Loop::time_point::min()); // Ditto.
EXPECT_THAT(evl.getTree().size(), 1); // Ditto.
out = evl.spin();
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(evl.getTree()[0U]->getDeadline().time_since_epoch(), 14'000ms);
EXPECT_THAT(1, evl.getTree().size()); // Second dropped.
EXPECT_TRUE(a);
EXPECT_THAT(a.value().deadline.time_since_epoch(), 13'000ms);
Expand Down Expand Up @@ -271,27 +270,26 @@ TEST(TestOlgaScheduler, EventLoopPoll)
EXPECT_FALSE(last_tp);
last_tp = arg.deadline;
});
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 110ms);
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 110ms);

// Check the type ID of return type poll().
const auto actual_type_id = decltype(evt)::_get_type_id_();
EXPECT_THAT(actual_type_id, ElementsAreArray(expected_type_id));
EXPECT_THAT(decltype(evt)::_get_type_id_(), ElementsAreArray(event_type_id));

SteadyClockMock::advance(30ms);
EXPECT_THAT(SteadyClockMock::now().time_since_epoch(), 130ms);
(void) evl.spin();
EXPECT_TRUE(last_tp);
EXPECT_THAT(last_tp.value().time_since_epoch(), 110ms);
last_tp.reset();
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 140ms); // Skipped ahead!
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 140ms); // Skipped ahead!

SteadyClockMock::advance(70ms);
EXPECT_THAT(SteadyClockMock::now().time_since_epoch(), 200ms);
(void) evl.spin();
EXPECT_TRUE(last_tp);
EXPECT_THAT(last_tp.value().time_since_epoch(), 140ms);
last_tp.reset();
EXPECT_THAT(evl.getTree()[0U]->getDeadline().value().time_since_epoch(), 210ms); // Skipped ahead!
EXPECT_THAT(evl.getTree()[0U]->getDeadline().time_since_epoch(), 210ms); // Skipped ahead!
}

TEST(TestOlgaScheduler, EventLoopDefer_single_overdue)
Expand All @@ -304,8 +302,7 @@ TEST(TestOlgaScheduler, EventLoopDefer_single_overdue)
auto evt = evl.defer(SteadyClockMock::now() + 1000ms, [](const auto&) {});

// Check the type ID of return type defer().
const auto actual_type_id = decltype(evt)::_get_type_id_();
EXPECT_THAT(actual_type_id, ElementsAreArray(expected_type_id));
EXPECT_THAT(decltype(evt)::_get_type_id_(), ElementsAreArray(event_type_id));

// 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.
Expand Down
Loading