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

8.0 Proposal (internal): Use IValueTaskSource for RPC continuations awaits #966

Open
bollhals opened this issue Oct 30, 2020 · 4 comments
Open
Assignees
Milestone

Comments

@bollhals
Copy link
Contributor

In order to prepare for the async interface, one part of the whole library is the RpcContinuationQueue. With the current implementation we can not support async waiting on completion.
=> I propose to use a IValueTaskSource and create a class that handles this.

This way we could set the reply with one method and have another which you could use to wait.

void SetReply(in IncomingCommand cmd)
ValueTask<IncomingCommand> GetReplyAsync()

instead of the current (used my latest branch to experiment)

IncomingCommand GetReply()
void HandleCommand(in IncomingCommand cmd)

With this implementation we can avoid a lot of allocations for an RPC await.

I have a very basic working prototype up and running locally.
If the extended functionality (timeout, shutdown) is fully implmented and working, I'll set up a PR for the current sync API with GetReplyAsync().AsTask.GetAwaiter().GetResult(). in every public API. => Async under the hood, so we'd ready to switch.

@stebet
Copy link
Collaborator

stebet commented Oct 30, 2020

I already have a working type in the async branch, which uses/reuses ValueTasks from a pooled resource. You can use that as a reference as well. It also uses locking to guarantee that only one operation is outstanding per model. See here: https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/full-async/projects/RabbitMQ.Client/client/impl/RpcContinuationQueue.cs

@michaelklishin michaelklishin changed the title 7.0 Proposal (internal): Use IValueTaskSource for RPC continuations awaits 8.0 Proposal (internal): Use IValueTaskSource for RPC continuations awaits Nov 6, 2020
@michaelklishin
Copy link
Member

Moving to 8.0 so that we can ship a 7.0 with a smaller set of breaking API changes if necessary.

@lukebakken
Copy link
Contributor

I believe that the AsyncRpcContinuation class and subclasses effectively implement this suggestion.

@stebet
Copy link
Collaborator

stebet commented Jan 3, 2024

I believe that the AsyncRpcContinuation class and subclasses effectively implement this suggestion.

They are TaskCompletionSource based which does allocate where IValueTaskSource doesn't. It's not a huge issue though and something I can look at later.

@lukebakken lukebakken modified the milestones: 7.0.0, 8.0.0 Jan 3, 2024
@lukebakken lukebakken reopened this Jan 3, 2024
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

No branches or pull requests

4 participants