search

Azure / azure-service-bus-dotnet

☁️ .NET Standard client library for Azure Service Bus
https://azure.microsoft.com/services/service-bus
Other
235 stars 120 forks source link

Provide a ValueTask return type for QueueClient.RegisterMessageHandler #609

Closed ghost closed 5 years ago

ghost commented 5 years ago

Based on Stephen Toub's topic regarding ValueTask here it would be great considering adding/replacing the QueueClient.RegisterMessageHandler return type as a ValueTask, which could vastly improve performances by reducing the amount of task allocations as the number of messages received grows.

SeanFeldman commented 5 years ago

@bbougot are you referring to the registered handler Func using RegisterMessageHandler(Func<Message, CancellationToken, Task> handler, ...)?

(This will also require .NET Standard 2.1 or System.Threading.Tasks.Extensions package.)

ghost commented 5 years ago

Yes absolutely.

SeanFeldman commented 5 years ago

In that case it could be also applied on exceptionReceivedHandler (that is only awaited internally) to be converted from Func<ExceptionReceivedEventArgs, Task> to Func<ExceptionReceivedEventArgs, ValueTask> as well.

SeanFeldman commented 5 years ago

I've created a spike PR #611, but it really should be tested for performance improvements to assume using ValueTask will improve any performance. Would you like to take a stab at verifying performance impact @bbougot?

ghost commented 5 years ago

@SeanFeldman Bench setup detailed here.

SeanFeldman commented 5 years ago

Given the findings in the PR #611 and the fact that ValueTask should not be surfaced to the public API, I'm going to close this issue. Should you disagree with the decision @bbougot, please re-open and provide the reasons to continue on this route. Thanks.