SslStream doesn't amortize its ValueTask ReadAsync calls

[benaadams]

In usage a SslStream normally has very many ReadAsync calls made on it. (Use case WebSockets/SignalR over TLS)

Using the ValueTask overloads for SslStream read allocates 2x AsyncStateMachineBox per read (when data is not immediately available)

However it could allocate once; backing with an IValueTaskSource objects that get allocated for the first async and reused when the read goes async again.

[stephentoub]

Difficult to do manually, and will complicate the code significantly. Unless you're planning to tackle this soon and find a way to keep it manageable in the process, I suggest you close the issue, or it is just going to sit open for a very long time.

[benaadams]

and find a way to keep it manageable in the process

I'm looking to see if can use IAsyncEnumerable as adding the new type that way is very simple (changing return to yield return).

However, I'm having some issue then gluing that to the IValueTaskSource<int> as its a ValueTask<bool>, so also need to go via a transform that converts one callback to the other.

[benaadams]

*Am trying it on HttpRequestStream first dotnet/aspnetcore#11940

[karelz]

Triage: @wfurt is working on SslStream redesign. If we still have perf opportunity after it is done, we may reopen this issue.
Closing until then.

[stephentoub]

working on SslStream redesign. If we still have perf opportunity after it is done, we may reopen this issue

@wfurt's changes are not affecting this.

[wfurt]

what should we do about this @stephentoub ? you suggested to close this earlier. Do you have sense how much this can help?

[stephentoub]

It's fine closing it, I just wanted to point out that this opportunity is still relevant after your changes. I'm not sure how much it would help. An easy way to get some sense would be to turn on the feature flags I merged in dotnet/coreclr#26310, and run a benchmark around SslStream, to see what impact it has on allocation and throughput, if any.