refactor(agnocastlib): remove event-specific logic from agnocast_epoll.cpp and .hpp#1275
Open
refactor(agnocastlib): remove event-specific logic from agnocast_epoll.cpp and .hpp#1275
agnocast_epoll.cpp and .hpp#1275Conversation
Closed
6 tasks
agnocast_epoll.cpp and .hpp
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Agnocast’s epoll handling to decouple event-specific logic from the epoll loop and make epoll updates per-executor, improving extensibility for new epoll event types.
Changes:
- Introduces an
EpollManager+EpollEventHandlerinterface and moves per-event handling into dedicated handler classes/files. - Replaces the global
need_epoll_updatesmechanism with a per-executorEpollUpdateDispatcher/EpollUpdateTrackernotification model. - Adds new unit/integration tests covering epoll data packing/unpacking and epoll-update behavior across executor lifecycle orderings.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/agnocastlib/src/agnocast_epoll.cpp | Implements EpollManager and routes events to handler objects. |
| src/agnocastlib/include/agnocast/agnocast_epoll.hpp | Defines handler interface, EpollManager, and handler array types. |
| src/agnocastlib/include/agnocast/agnocast_epoll_event.hpp | Adds epoll data packing/unpacking and event type enum. |
| src/agnocastlib/src/agnocast_epoll_update_dispatcher.cpp | Implements per-executor epoll update dispatcher/tracker. |
| src/agnocastlib/include/agnocast/agnocast_epoll_update_dispatcher.hpp | Declares dispatcher/tracker API used by executors and registrars. |
| src/agnocastlib/src/agnocast_callback_info.cpp | Moves subscription epoll prepare/handle logic into SubscriptionEventHandler. |
| src/agnocastlib/include/agnocast/agnocast_callback_info.hpp | Adds callback-ID max constants and declares SubscriptionEventHandler. |
| src/agnocastlib/src/agnocast_timer_info.cpp | Moves timer/clock epoll prepare/handle logic into TimerEventHandler/ClockEventHandler; switches to dispatcher notifications. |
| src/agnocastlib/include/agnocast/agnocast_timer_info.hpp | Adds timer-ID max constants and declares new timer/clock handler classes. |
| src/agnocastlib/src/agnocast_executor.cpp | Switches executor epoll usage to EpollManager and per-executor update tracker. |
| src/agnocastlib/include/agnocast/agnocast_executor.hpp | Stores EpollManager and EpollUpdateTracker in base executor. |
| src/agnocastlib/src/agnocast_single_threaded_executor.cpp | Uses EpollUpdateTracker::take_update_request() to trigger epoll preparation. |
| src/agnocastlib/src/agnocast_multi_threaded_executor.cpp | Uses EpollUpdateTracker::take_update_request() to trigger epoll preparation. |
| src/agnocastlib/src/node/agnocast_only_executor.cpp | Refactors Agnocast-only executor to use EpollManager + per-executor update tracker; requests updates on node/group changes. |
| src/agnocastlib/include/agnocast/node/agnocast_only_executor.hpp | Replaces raw epoll_fd_ with EpollManager and adds tracker member. |
| src/agnocastlib/src/node/agnocast_only_single_threaded_executor.cpp | Uses tracker-driven epoll updates and EpollManager::prepare_epoll(). |
| src/agnocastlib/src/node/agnocast_only_multi_threaded_executor.cpp | Uses tracker-driven epoll updates and EpollManager::prepare_epoll(). |
| src/agnocastlib/test/unit/test_mocked_agnocast.cpp | Updates ID overflow boundary tests to use new MAX_* constants. |
| src/agnocastlib/test/unit/test_agnocast_epoll_update.cpp | Adds unit tests for dispatcher/tracker update semantics. |
| src/agnocastlib/test/unit/test_agnocast_epoll_event.cpp | Adds unit tests for epoll data packing/unpacking. |
| src/agnocastlib/test/integration/test_agnocast_epoll_update.cpp | Adds integration tests validating epoll updates across executor operation order permutations. |
| src/agnocastlib/CMakeLists.txt | Registers new source/test files and adds a new integration test target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously, all event-specific logic was hardcoded in `agnocast_epoll.cpp` and `.hpp`. This caused dependency issues and made it difficult to add new event types. This commit refactors the code by categorizing events and moving their implementations into separate source files. Key changes include: - Change epoll_data format from u32 to u64 to hold both event kind and local identifier. - Introduce `EpollManager` to manage and dispatch events from epoll. - Introduce `EpollEventHandler` as an abstract base class for specific event handlers. Relates to: #969 Signed-off-by: Takumi Jin <primenumber_2_3_5@yahoo.co.jp>
Signed-off-by: Takumi Jin <primenumber_2_3_5@yahoo.co.jp>
Signed-off-by: Takumi Jin <primenumber_2_3_5@yahoo.co.jp>
4c668d1 to
756505b
Compare
…declaration Signed-off-by: Takumi Jin <primenumber_2_3_5@yahoo.co.jp>
17198f4 to
5854394
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR refactors
agnocast_epoll.cppand.hppto improve code maintainability and make it easier to add new events in the future.Problem
Previously, the event-specific logic for handling epoll events was written directly inside
agnocast_epoll.cppand.hpp. This caused high code dependency. Every time a new event was added, the dependency grew, making the code harder to manage.Changes
agnocast_epoll.cppand.hpp.EpollEventHandler, an abstract base class. The logic for each event is now implemented in a derived class that inherits from this base class.EpollManager, a new class to manage epoll and call these event handlers.Related links
close #969
#1271
How was this PR tested?
bash scripts/test/e2e_test_1to1.bash(required)bash scripts/test/e2e_test_2to2.bash(required)bash scripts/test/run_requires_kernel_module_tests.bash(required)Notes for reviewers
Version Update Label (Required)
Please add exactly one of the following labels to this PR:
need-major-update: User API breaking changesneed-minor-update: Internal API breaking changes (heaphook/kmod/agnocastlib compatibility)need-patch-update: Bug fixes and other changesImportant notes:
need-major-updateorneed-minor-update, please include this in the PR title as well.fix(foo)[needs major version update]: barorfeat(baz)[needs minor version update]: quxrun-build-testlabel. The PR can only be merged after the build tests pass.See CONTRIBUTING.md for detailed versioning rules.