Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proto-layer endpoint/connection communication #1726

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

Ralith
Copy link
Collaborator

@Ralith Ralith commented Dec 17, 2023

Gives us freedom to adopt more specialized communication strategies (e.g. atomics or fine-grained locks) between endpoints and connections. These will need to be implemented at the proto layer, because optimal strategies will be sensitive to the specific data proto-private being communicated, e.g. NeedIdentifiers as an atomic counter.

Simplified take on #1650, pushing cheeky data structures to future work.

This was only needed to determine the base time for the next
CID retirement timer, for which the marginal delay caused by fetching
a new `Instant::now()` on demand is harmless.
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind writing a more detailed description of how this PR makes this more modular? I skimmed the commits but the new structure (or, how the structure changes) is still a bit hazy.

quinn/src/connection.rs Outdated Show resolved Hide resolved
quinn/Cargo.toml Outdated Show resolved Hide resolved
Groundwork for replacing generic message-passing with lighter
event-representation-aware communication strategies.
Groundwork for direct connection -> driver wake-ups, and yields one
less piece of state inside a monolithic mutex.
Groundwork for replacing generic message-passing with lighter
event-representation-aware communication strategies.
@Ralith
Copy link
Collaborator Author

Ralith commented Dec 18, 2023

The main effect of this PR is to replace the requirement that quinn-proto users pass endpoint and connection events around themselves with the requirement that users trigger wakeups when events have occurred, with quinn-proto managing the actual message-passing internally. This is not necessarily more modular, but could be useful because it allows future work to replace naive message-passing with more specialized structures as in #1650. A more specialized communications structure could be lighter: an AtomicUsize counter is much lighter than message-passing for requesting new connection IDs and has constant memory overhead, for example. We still need to dispatch wakeups, but unlike messages, those are idempotent and hence don't need to be buffered, which might simplify downstream code.

It's not obvious that the benefit here is significant as it stands. quinn wasn't simplified, because it still needs the same channels for other purposes. Perhaps those can be addressed in future work; e.g. quinn::ConnectionEvent could probably be reduced to a set of shared flags.

My real goal is to work towards fine-grained interior mutability in quinn-proto, particularly Endpoint, to eventually support parallel endpoint drivers. I don't think this PR moves us in that direction.

This is also a step towards resolving #1131, but the real substance of the hazard described there is in handling datagrams, which is unchanged, and could be addressed without this PR.

This effort was ultimately exploratory, and I'm open to shelving it until it's more strongly motivated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants