feat: async event loop #326
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an async event loop system to synchronize events from different threads, replacing the previous unsynchronized event handling approach. Previously, events from other threads were not being scheduled because there was no mechanism to update the loop state when they arrived.
Key changes:
- Implements
EventLoopwith async notification system for cross-thread event scheduling - Replaces direct
xev.Loopusage with wrappedEventLoopacross network and node layers - Removes the boolean
scheduleOnLoopparameter from gossip handling
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkgs/utils/src/lib.zig | Exports new EventLoop type from utils package |
| pkgs/utils/src/event_loop.zig | New async event loop implementation with mutex-protected work queue and comprehensive tests |
| pkgs/node/src/utils.zig | Removes old EventLoop implementation that lacked thread synchronization |
| pkgs/node/src/node.zig | Updates test to use new EventLoop wrapper |
| pkgs/node/src/clock.zig | Replaces direct loop reference with EventLoop, removes redundant loop.run() call |
| pkgs/network/src/mock.zig | Updates to use EventLoop and removes scheduleOnLoop parameter |
| pkgs/network/src/interface.zig | Refactors gossip handling to use work queue instead of timer-based scheduling |
| pkgs/network/src/ethlibp2p.zig | Updates to use EventLoop and removes scheduleOnLoop parameter |
| pkgs/cli/src/node.zig | Manages EventLoop lifecycle with proper initialization and cleanup |
| pkgs/cli/src/main.zig | Updates all command handlers to use new EventLoop |
| build.zig | Adds xev dependency to utils module |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pkgs/utils/src/event_loop.zig
Outdated
| /// This method signals the event loop to stop, sends a notification for pending work, | ||
| /// and attempts to process it immediately with a best-effort loop run. | ||
| /// Should be called before deinit(). | ||
| pub fn stop(self: *Self) void { |
There was a problem hiding this comment.
we can add a stop async too, the best practice of xev.loop usage is https://github.com/ghostty-org/ghostty/blob/fb5b8d7968e6d760b53785ba169c751de75ac08d/src/termio/Thread.zig#L240
| pub const Node = struct { | ||
| loop: xev.Loop, | ||
| loop: *xev.Loop, | ||
| event_loop: *zeam_utils.EventLoop, |
There was a problem hiding this comment.
shall we just keep event_loop since it already has the loop inside?
There was a problem hiding this comment.
Makes sense, I'll make event_loop the owner of loop
|
Almost good, just some small suggestion |
Closes #312
Introduces a new async event loop system that handles events from different threads using async signalling, replacing the previous event handling approach across the network and node layers.
Previously, there was no synchronization mechanism between threads, so the event loop state was not updated when events arrived from other threads. This caused the loop to continue executing on stale state, and events from other threads were not being scheduled.