Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase to isolate simulation-specific code into a new pulsebeam-core crate, removing the sim feature flag from individual crates and centralizing conditional compilation logic. The PR also introduces a UdpMode enum to distinguish between batch and scalar UDP transport modes.
- Introduces
pulsebeam-corecrate to centralize simulation-specific networking abstractions - Adds
UdpModeenum toTransportto differentiate between batch (sendmmsg/GSO) and scalar (send_to) UDP modes - Removes
simfeature flags frompulsebeam,pulsebeam-runtime, andpulsebeam-agentcrates
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pulsebeam-core/Cargo.toml | New crate manifest with optional sim feature that enables turmoil and axum dependencies |
| pulsebeam-core/src/lib.rs | Module declaration for the new core crate |
| pulsebeam-core/src/net.rs | Conditional exports for networking types based on sim feature, including TurmoilListener wrapper for axum integration |
| pulsebeam/Cargo.toml | Removes sim feature and turmoil dependency, adds pulsebeam-core dependency |
| pulsebeam/src/node.rs | Removes turmoil_listener module, uses TcpListener from pulsebeam-core, updates UDP binding to use UdpMode::Batch |
| pulsebeam/src/controller.rs | Updates Transport::Udp pattern match to handle new UdpMode parameter |
| pulsebeam/src/participant/core.rs | Updates Transport::Udp pattern match to handle new UdpMode parameter |
| pulsebeam-runtime/Cargo.toml | Removes sim feature and turmoil dependency, adds pulsebeam-core dependency |
| pulsebeam-runtime/src/net/mod.rs | Adds UdpMode enum, updates Transport enum to include UdpMode, adds UdpScalar variants to UnifiedSocket enums, implements udp_scalar binding logic |
| pulsebeam-runtime/src/net/udp.rs | Updates to use UdpMode in Transport enum, sets transport mode in RecvPacketBatch |
| pulsebeam-runtime/src/net/udp_scalar.rs | New module implementing scalar UDP operations using pulsebeam-core networking primitives |
| pulsebeam-runtime/src/net/tcp.rs | Updates imports to use TcpListener and TcpStream from pulsebeam-core |
| pulsebeam-agent/Cargo.toml | Removes sim feature and turmoil dependency, adds pulsebeam-core dependency |
| pulsebeam-agent/src/actor.rs | Updates imports to use UdpSocket from pulsebeam-core |
| pulsebeam-cli/Cargo.toml | Adds pulsebeam-core dependency |
| pulsebeam-cli/src/main.rs | Updates import to use UdpSocket from pulsebeam-core |
| pulsebeam-simulator/Cargo.toml | Removes sim features from dependencies, adds pulsebeam-core with sim feature |
| Cargo.toml | Adds pulsebeam-core to workspace members, removes default-members section |
| Cargo.lock | Reflects dependency changes across all crates |
| Makefile | Simplifies build target, adds separate profile target, adds test-all and test-sim targets |
| .github/workflows/ci.yml | Updates to use make commands for testing instead of direct cargo commands |
Comments suppressed due to low confidence (1)
pulsebeam-runtime/src/net/udp_scalar.rs:57
- The transport mode should be
UdpMode::Scalarinstead ofUdpMode::Batch. This is in the scalar UDP module (udp_scalar.rs) which uses standardsend_tofor lower latency, so the transport field should reflect that it's using scalar mode, not batch mode.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stride: m.stride, | ||
| len: m.len, | ||
| transport: Transport::Udp, | ||
| transport: Transport::Udp(UdpMode::Scalar), |
There was a problem hiding this comment.
The transport mode should be UdpMode::Batch instead of UdpMode::Scalar. This is in the batch UDP module (udp.rs) which uses sendmmsg/GSO for higher throughput, so the transport field should reflect that it's using batch mode, not scalar mode.
| transport: Transport::Udp(UdpMode::Scalar), | |
| transport: Transport::Udp(UdpMode::Batch), |
No description provided.