aspnet / HttpAbstractions

[Archived] HTTP abstractions such as HttpRequest, HttpResponse, and HttpContext, as well as common web utilities. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
382 stars 193 forks source link

[PoC] HttpResponseStreamWriter on Stream on Pipe #1001

Closed benaadams closed 6 years ago

benaadams commented 6 years ago

No additional buffers or ArrayPool buffers when Stream implements IDuplexPipe

Also current HttpResponseStreamWriter never ever called Flush; even if the user called it or when disposed O_o

Added

/cc @davidfowl @rynowak

benaadams commented 6 years ago

Needs the WriteAsync in pipelines that takes a cancellation token.

This means if Kestrel implemented IDuplexPipe or better IPipeWriter https://github.com/dotnet/corefx/issues/27268 on its ResponseStream the intermediate arrays could go away and encoding could be written directly to the Pipe; and MVC could take advantage of this with no changes.

davidfowl commented 6 years ago

This is optimized for synchronous writing of utf8 text to an underlying IBufferWriter<byte>, https://github.com/aspnet/SignalR/blob/f7fc2647de0f91fbe6322aec95b334ad1437f427/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/Utf8BufferTextWriter.cs. We could try using it here as an alternative to what MVC uses (or try gutting this impl).

muratg commented 6 years ago

@davidfowl @Tratcher is this still relevant/useful in your opinion? If so, let's move it forward. If not, let's get it closed.

Ben, thanks for the PR!

muratg commented 6 years ago

Triage decision: We’re closing this PR because we don’t feel that this change is a good fit for the product at this time.

We thank you for the contribution and look forward to collaborating more in the future.