From d05d1f3fb8272ebe48407cfa42cfe9c938683106 Mon Sep 17 00:00:00 2001 From: Richard Peters Date: Sat, 13 Jan 2024 14:03:18 +0100 Subject: [PATCH] feat: add ExecuteUntil to event dispatchers, add EventDispatcherThreadAware (#526) * infra/event/EventDispatcher{WithWeakPtr}: Add ExecuteUntil * Add infra/event/EventDispatcherThreadAware * Apply suggestions from code review Co-authored-by: Daan Timmer <8293597+daantimmer@users.noreply.github.com> * Update infra/event/test/TestEventDispatcherThreadAware.cpp Co-authored-by: Daan Timmer <8293597+daantimmer@users.noreply.github.com> --------- Co-authored-by: Daan Timmer <8293597+daantimmer@users.noreply.github.com> --- infra/event/CMakeLists.txt | 7 + infra/event/EventDispatcher.cpp | 13 ++ infra/event/EventDispatcher.hpp | 1 + infra/event/EventDispatcherThreadAware.cpp | 24 +++ infra/event/EventDispatcherThreadAware.hpp | 28 +++ infra/event/EventDispatcherWithWeakPtr.cpp | 13 ++ infra/event/EventDispatcherWithWeakPtr.hpp | 1 + infra/event/test/CMakeLists.txt | 5 +- ...entHandler.cpp => TestEventDispatcher.cpp} | 80 ++++++++- .../test/TestEventDispatcherThreadAware.cpp | 47 +++++ .../test/TestEventDispatcherWithWeakPtr.cpp | 161 ++++++++++++++++++ .../test/TestEventHandlerWithWeakPtr.cpp | 92 ---------- 12 files changed, 371 insertions(+), 101 deletions(-) create mode 100644 infra/event/EventDispatcherThreadAware.cpp create mode 100644 infra/event/EventDispatcherThreadAware.hpp rename infra/event/test/{TestEventHandler.cpp => TestEventDispatcher.cpp} (52%) create mode 100644 infra/event/test/TestEventDispatcherThreadAware.cpp create mode 100644 infra/event/test/TestEventDispatcherWithWeakPtr.cpp delete mode 100644 infra/event/test/TestEventHandlerWithWeakPtr.cpp diff --git a/infra/event/CMakeLists.txt b/infra/event/CMakeLists.txt index bd13f8ddc..75e043c0d 100644 --- a/infra/event/CMakeLists.txt +++ b/infra/event/CMakeLists.txt @@ -19,5 +19,12 @@ target_sources(infra.event PRIVATE SystemStateManager.hpp ) +if (EMIL_HOST_BUILD) + target_sources(infra.event PRIVATE + EventDispatcherThreadAware.cpp + EventDispatcherThreadAware.hpp + ) +endif() + add_subdirectory(test) add_subdirectory(test_helper) diff --git a/infra/event/EventDispatcher.cpp b/infra/event/EventDispatcher.cpp index c7de58d42..a372a973b 100644 --- a/infra/event/EventDispatcher.cpp +++ b/infra/event/EventDispatcher.cpp @@ -46,6 +46,19 @@ namespace infra {} } + void EventDispatcherWorkerImpl::ExecuteUntil(const infra::Function& predicate) + { + while (!predicate()) + { + ExecuteAllActions(); + + if (predicate()) + break; + + Idle(); + } + } + void EventDispatcherWorkerImpl::ExecuteFirstAction() { if (scheduledActions[scheduledActionsPopIndex].second) diff --git a/infra/event/EventDispatcher.hpp b/infra/event/EventDispatcher.hpp index 568f10c13..586b7e384 100644 --- a/infra/event/EventDispatcher.hpp +++ b/infra/event/EventDispatcher.hpp @@ -39,6 +39,7 @@ namespace infra void Run(); void ExecuteAllActions(); + void ExecuteUntil(const infra::Function& predicate); protected: virtual void RequestExecution(); diff --git a/infra/event/EventDispatcherThreadAware.cpp b/infra/event/EventDispatcherThreadAware.cpp new file mode 100644 index 000000000..0711064cc --- /dev/null +++ b/infra/event/EventDispatcherThreadAware.cpp @@ -0,0 +1,24 @@ +#include "infra/event/EventDispatcherThreadAware.hpp" + +namespace infra +{ + void EventDispatcherThreadAwareWorker::RequestExecution() + { + std::lock_guard lock{ mutex }; + + ready = true; + condition.notify_one(); + } + + void EventDispatcherThreadAwareWorker::Idle() + { + std::unique_lock lock{ mutex }; + + condition.wait(lock, [this]() + { + return ready; + }); + + ready = false; + } +} diff --git a/infra/event/EventDispatcherThreadAware.hpp b/infra/event/EventDispatcherThreadAware.hpp new file mode 100644 index 000000000..8e424c80f --- /dev/null +++ b/infra/event/EventDispatcherThreadAware.hpp @@ -0,0 +1,28 @@ +#ifndef INFRA_EVENT_DISPATCHER_THREAD_AWARE_HPP +#define INFRA_EVENT_DISPATCHER_THREAD_AWARE_HPP + +#include "infra/event/EventDispatcherWithWeakPtr.hpp" +#include +#include + +namespace infra +{ + class EventDispatcherThreadAwareWorker + : public infra::EventDispatcherWithWeakPtrWorker + { + public: + using EventDispatcherWithWeakPtrWorker::EventDispatcherWithWeakPtrWorker; + + void RequestExecution() override; + void Idle() override; + + private: + std::condition_variable condition; + std::mutex mutex; + bool ready{ false }; + }; + + using EventDispatcherThreadAware = EventDispatcherWithWeakPtrConnector; +} + +#endif diff --git a/infra/event/EventDispatcherWithWeakPtr.cpp b/infra/event/EventDispatcherWithWeakPtr.cpp index eade18bc3..8aff09c16 100644 --- a/infra/event/EventDispatcherWithWeakPtr.cpp +++ b/infra/event/EventDispatcherWithWeakPtr.cpp @@ -46,6 +46,19 @@ namespace infra {} } + void EventDispatcherWithWeakPtrWorker::ExecuteUntil(const infra::Function& predicate) + { + while (!predicate()) + { + ExecuteAllActions(); + + if (predicate()) + break; + + Idle(); + } + } + bool EventDispatcherWithWeakPtrWorker::IsIdle() const { return !scheduledActions[scheduledActionsPopIndex].second; diff --git a/infra/event/EventDispatcherWithWeakPtr.hpp b/infra/event/EventDispatcherWithWeakPtr.hpp index 61c3cc97c..b91168ec0 100644 --- a/infra/event/EventDispatcherWithWeakPtr.hpp +++ b/infra/event/EventDispatcherWithWeakPtr.hpp @@ -68,6 +68,7 @@ namespace infra void Run(); void ExecuteAllActions(); + void ExecuteUntil(const infra::Function& predicate); protected: virtual void RequestExecution(); diff --git a/infra/event/test/CMakeLists.txt b/infra/event/test/CMakeLists.txt index 9bbf41ff2..539a2f14e 100644 --- a/infra/event/test/CMakeLists.txt +++ b/infra/event/test/CMakeLists.txt @@ -10,8 +10,9 @@ target_link_libraries(infra.event_test PUBLIC target_sources(infra.event_test PRIVATE TestClaimableResource.cpp - TestEventHandler.cpp - TestEventHandlerWithWeakPtr.cpp + TestEventDispatcher.cpp + TestEventDispatcherWithWeakPtr.cpp + TestEventDispatcherThreadAware.cpp TestQueueForOneReaderOneIrqWriter.cpp TestSystemStateManager.cpp ) diff --git a/infra/event/test/TestEventHandler.cpp b/infra/event/test/TestEventDispatcher.cpp similarity index 52% rename from infra/event/test/TestEventHandler.cpp rename to infra/event/test/TestEventDispatcher.cpp index a312b1aca..448f06e62 100644 --- a/infra/event/test/TestEventHandler.cpp +++ b/infra/event/test/TestEventDispatcher.cpp @@ -9,10 +9,7 @@ class EventDispatcherTest , public infra::EventDispatcherFixture {}; -TEST_F(EventDispatcherTest, TestConstruction) -{} - -TEST_F(EventDispatcherTest, TestSchedule) +TEST_F(EventDispatcherTest, scheduled_action_is_executed) { infra::MockCallback callback; EXPECT_CALL(callback, callback()); @@ -24,7 +21,7 @@ TEST_F(EventDispatcherTest, TestSchedule) ExecuteAllActions(); } -TEST_F(EventDispatcherTest, TestScheduleTwice) +TEST_F(EventDispatcherTest, two_actions_are_executed) { infra::MockCallback callback; EXPECT_CALL(callback, callback()).Times(2); @@ -40,7 +37,7 @@ TEST_F(EventDispatcherTest, TestScheduleTwice) ExecuteAllActions(); } -TEST_F(EventDispatcherTest, TestExecuteOneEvent) +TEST_F(EventDispatcherTest, ExecuteFirstAction_executes_first_action) { infra::MockCallback callback; @@ -63,6 +60,75 @@ TEST_F(EventDispatcherTest, TestExecuteOneEvent) ExecuteFirstAction(); } +class EventDispatcherMock + : public infra::EventDispatcher::WithSize<50> +{ +public: + MOCK_METHOD(void, RequestExecution, (), (override)); + MOCK_METHOD(void, Idle, (), (override)); +}; + +class EventDispatcherDetailedTest + : public testing::Test + , public EventDispatcherMock +{}; + +TEST_F(EventDispatcherDetailedTest, ExecuteUntil_executes_until_first_action_sets_predicate_true) +{ + bool done = false; + + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcher::Instance().Schedule([&]() + { + done = true; + }); + + ExecuteUntil([&]() + { + return done; + }); +} + +TEST_F(EventDispatcherDetailedTest, ExecuteUntil_executes_until_idle_sets_predicate_true) +{ + bool done = false; + + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcher::Instance().Schedule([&]() {}); + + EXPECT_CALL(*this, Idle()).WillOnce(testing::Invoke([&]() + { + done = true; + })); + + ExecuteUntil([&]() + { + return done; + }); +} + +TEST_F(EventDispatcherDetailedTest, ExecuteUntil_executes_until_second_action_sets_predicate_true) +{ + bool done = false; + + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcher::Instance().Schedule([&]() {}); + + EXPECT_CALL(*this, Idle()).WillOnce(testing::Invoke([&]() + { + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcher::Instance().Schedule([&]() + { + done = true; + }); + })); + + ExecuteUntil([&]() + { + return done; + }); +} + bool helper1turn = true; void helper1() @@ -77,7 +143,7 @@ void helper2() helper1turn = true; } -TEST_F(EventDispatcherTest, TestPerformance) +TEST_F(EventDispatcherTest, execute_a_lot) { for (int j = 0; j != 10; ++j) { diff --git a/infra/event/test/TestEventDispatcherThreadAware.cpp b/infra/event/test/TestEventDispatcherThreadAware.cpp new file mode 100644 index 000000000..b23e9325c --- /dev/null +++ b/infra/event/test/TestEventDispatcherThreadAware.cpp @@ -0,0 +1,47 @@ +#include "infra/event/EventDispatcherThreadAware.hpp" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include + +class EventDispatcherThreadAwareIdleCounting + : public infra::EventDispatcherThreadAware::WithSize<50> +{ +protected: + void Idle() override + { + ++idleCount; + infra::EventDispatcherThreadAware::WithSize<50>::Idle(); + } + +public: + int idleCount = 0; +}; + +class EventDispatcherThreadAwareTest + : public testing::Test + , public EventDispatcherThreadAwareIdleCounting +{}; + +TEST_F(EventDispatcherThreadAwareTest, scheduled_action_is_executed) +{ + bool done = false; + + std::thread t([&]() + { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + infra::EventDispatcher::Instance().Schedule([&]() + { + done = true; + }); + }); + + ExecuteUntil([&]() + { + return done; + }); + + EXPECT_THAT(idleCount, testing::Eq(1)); + + t.join(); +} diff --git a/infra/event/test/TestEventDispatcherWithWeakPtr.cpp b/infra/event/test/TestEventDispatcherWithWeakPtr.cpp new file mode 100644 index 000000000..e6db6d711 --- /dev/null +++ b/infra/event/test/TestEventDispatcherWithWeakPtr.cpp @@ -0,0 +1,161 @@ +#include "infra/event/EventDispatcherWithWeakPtr.hpp" +#include "infra/event/test_helper/EventDispatcherWithWeakPtrFixture.hpp" +#include "infra/util/SharedObjectAllocatorFixedSize.hpp" +#include "infra/util/test_helper/MockCallback.hpp" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +class EventDispatcherWithWeakPtrTest + : public testing::Test + , public infra::EventDispatcherWithWeakPtrFixture +{}; + +TEST_F(EventDispatcherWithWeakPtrTest, one_action_is_executed) +{ + infra::MockCallback callback; + EXPECT_CALL(callback, callback()); + + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this]() + { + callback.callback(); + }); + ExecuteAllActions(); +} + +TEST_F(EventDispatcherWithWeakPtrTest, two_actions_are_executed) +{ + infra::MockCallback callback; + EXPECT_CALL(callback, callback()).Times(2); + + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this]() + { + callback.callback(); + }); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this]() + { + callback.callback(); + }); + ExecuteAllActions(); +} + +TEST_F(EventDispatcherWithWeakPtrTest, ExecuteFirstAction_executes_first_action) +{ + infra::MockCallback callback; + + EXPECT_CALL(callback, callback()).Times(0); + ExecuteFirstAction(); + + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this]() + { + callback.callback(); + }); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this]() + { + callback.callback(); + }); + + EXPECT_CALL(callback, callback()).Times(1); + ExecuteFirstAction(); + + EXPECT_CALL(callback, callback()).Times(1); + ExecuteFirstAction(); +} + +TEST_F(EventDispatcherWithWeakPtrTest, scheduled_action_with_SharedPtr_is_executed) +{ + infra::MockCallback callback; + EXPECT_CALL(callback, callback()); + + infra::SharedObjectAllocatorFixedSize::WithStorage<2> allocator; + infra::SharedPtr object = allocator.Allocate(); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this](const infra::SharedPtr& object) + { + callback.callback(); + }, + object); + ExecuteAllActions(); +} + +TEST_F(EventDispatcherWithWeakPtrTest, scheduled_action_with_destructed_SharedPtr_is_not_executed) +{ + testing::StrictMock> callback; + + infra::SharedObjectAllocatorFixedSize::WithStorage<2> allocator; + infra::SharedPtr object = allocator.Allocate(); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this](const infra::SharedPtr& object) + { + callback.callback(); + }, + object); + object = nullptr; + ExecuteAllActions(); +} + +class EventDispatcherWithWeakPtrMock + : public infra::EventDispatcherWithWeakPtr::WithSize<50> +{ +public: + MOCK_METHOD(void, RequestExecution, (), (override)); + MOCK_METHOD(void, Idle, (), (override)); +}; + +class EventDispatcherWithWeakPtrDetailedTest + : public testing::Test + , public EventDispatcherWithWeakPtrMock +{}; + +TEST_F(EventDispatcherWithWeakPtrDetailedTest, ExecuteUntil_executes_until_first_action_sets_predicate_true) +{ + bool done = false; + + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&]() + { + done = true; + }); + + ExecuteUntil([&]() + { + return done; + }); +} + +TEST_F(EventDispatcherWithWeakPtrDetailedTest, ExecuteUntil_executes_until_idle_sets_predicate_true) +{ + bool done = false; + + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&]() {}); + + EXPECT_CALL(*this, Idle()).WillOnce(testing::Invoke([&]() + { + done = true; + })); + + ExecuteUntil([&]() + { + return done; + }); +} + +TEST_F(EventDispatcherWithWeakPtrDetailedTest, ExecuteUntil_executes_until_second_action_sets_predicate_true) +{ + bool done = false; + + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&]() {}); + + EXPECT_CALL(*this, Idle()).WillOnce(testing::Invoke([&]() + { + EXPECT_CALL(*this, RequestExecution()); + infra::EventDispatcherWithWeakPtr::Instance().Schedule([&]() + { + done = true; + }); + })); + + ExecuteUntil([&]() + { + return done; + }); +} diff --git a/infra/event/test/TestEventHandlerWithWeakPtr.cpp b/infra/event/test/TestEventHandlerWithWeakPtr.cpp deleted file mode 100644 index 20c5bba7c..000000000 --- a/infra/event/test/TestEventHandlerWithWeakPtr.cpp +++ /dev/null @@ -1,92 +0,0 @@ -#include "infra/event/EventDispatcher.hpp" -#include "infra/event/test_helper/EventDispatcherWithWeakPtrFixture.hpp" -#include "infra/util/SharedObjectAllocatorFixedSize.hpp" -#include "infra/util/test_helper/MockCallback.hpp" -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -class EventDispatcherWithWeakPtrTest - : public testing::Test - , public infra::EventDispatcherWithWeakPtrFixture -{}; - -TEST_F(EventDispatcherWithWeakPtrTest, TestSchedule) -{ - infra::MockCallback callback; - EXPECT_CALL(callback, callback()); - - infra::EventDispatcher::Instance().Schedule([&callback, this]() - { - callback.callback(); - }); - ExecuteAllActions(); -} - -TEST_F(EventDispatcherWithWeakPtrTest, TestScheduleTwice) -{ - infra::MockCallback callback; - EXPECT_CALL(callback, callback()).Times(2); - - infra::EventDispatcher::Instance().Schedule([&callback, this]() - { - callback.callback(); - }); - infra::EventDispatcher::Instance().Schedule([&callback, this]() - { - callback.callback(); - }); - ExecuteAllActions(); -} - -TEST_F(EventDispatcherWithWeakPtrTest, TestExecuteOneEvent) -{ - infra::MockCallback callback; - - EXPECT_CALL(callback, callback()).Times(0); - ExecuteFirstAction(); - - infra::EventDispatcher::Instance().Schedule([&callback, this]() - { - callback.callback(); - }); - infra::EventDispatcher::Instance().Schedule([&callback, this]() - { - callback.callback(); - }); - - EXPECT_CALL(callback, callback()).Times(1); - ExecuteFirstAction(); - - EXPECT_CALL(callback, callback()).Times(1); - ExecuteFirstAction(); -} - -TEST_F(EventDispatcherWithWeakPtrTest, TestScheduleSharedPtr) -{ - infra::MockCallback callback; - EXPECT_CALL(callback, callback()); - - infra::SharedObjectAllocatorFixedSize::WithStorage<2> allocator; - infra::SharedPtr object = allocator.Allocate(); - infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this](const infra::SharedPtr& object) - { - callback.callback(); - }, - object); - ExecuteAllActions(); -} - -TEST_F(EventDispatcherWithWeakPtrTest, TestScheduleDestructedSharedPtr) -{ - testing::StrictMock> callback; - - infra::SharedObjectAllocatorFixedSize::WithStorage<2> allocator; - infra::SharedPtr object = allocator.Allocate(); - infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this](const infra::SharedPtr& object) - { - callback.callback(); - }, - object); - object = nullptr; - ExecuteAllActions(); -}