search

quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.85k stars 394 forks source link

Communicate endpoint events in -proto w/ an intrusive queue #1650

Closed Ralith closed 10 months ago

Ralith commented 1 year ago

Work towards #1576, https://github.com/quinn-rs/quinn/issues/599.

This replaces the requirement for callers to pass EndpointEvents around with shared memory managed by the proto layer, using a purpose-built lock-free intrusive mpsc queue called SharedList. This simplifies the proto API and may be more efficient, although endpoint events are not typically on a hot path.

Intensive review is required for the implementation of SharedList, which contains internal unsafety and relies on careful use of memory orderings for correctness. I couldn't find an off-the-shelf, well-audited data structure with similar semantics.

Alternative approaches to this API change include a std mpsc channel and the pre-existing EndpointEvent enum, or an mpsc channel of Arc<EndpointEvents> with an additional AtomicBool flag to minimize redundant messaging. These would avoid rolling our own data structure, at the cost using a more reviewed but needlessly complex and less specialized data structure. Further, SharedList may prove useful elsewhere within quinn: a similar queue-once MPSC pattern exists in streams, and might be leveraged to increase application parallelism when performing stream I/O (see discussion in https://github.com/quinn-rs/quinn/issues/1433).

Ralith commented 1 year ago

In retrospect this might be more gracefully posed by first refactoring to use an internal mpsc queue, then replacing the mpsc queue with the custom one. Let me know if that's worth investing time in.

Ralith commented 10 months ago

Closing in favor of #1726, though if we continue in that direction we may eventually want to port the intrusive queue over.