New setting to enable Kernel response buffering #418
Conversation
|
Items to discuss:
|
There was a problem hiding this comment.
Please move this PR to the dev branch.
Writing functional tests for this is going to be hard as there's no behavioral difference, at best we turn it on and make sure nothing breaks. At least verify the four major code paths: Write, WriteAsync, BeginWrite, and SendFileAsync.
| @@ -61,6 +61,15 @@ public ILogger Logger | |||
| /// </summary> | |||
| public bool ThrowWriteExceptions { get; set; } | |||
|
|
|||
| /// <summary> | |||
| /// Gets or sets a value that indicates whether to enable buffering of data in the kernel on a per-response basis. | |||
There was a problem hiding this comment.
enable buffering of response data in the kernel.
| /// Gets or sets a value that indicates whether to enable buffering of data in the kernel on a per-response basis. | ||
| /// It should be used by an application doing synchronous I/O or by an application doing asynchronous I/O with | ||
| /// no more than one outstanding send at a time. | ||
| /// Applications that use asynchronous I/O and that may have more than one send outstanding at a time should not use this flag. |
| /// It should be used by an application doing synchronous I/O or by an application doing asynchronous I/O with | ||
| /// no more than one outstanding send at a time. | ||
| /// Applications that use asynchronous I/O and that may have more than one send outstanding at a time should not use this flag. | ||
| /// Enabling this can results in higher CPU and memory usage by Http.Sys. |
There was a problem hiding this comment.
and improve throughput over high latency connections
|
As for configuring this on a per response basis, we have ways of doing that but I don't see an immediate need for it. |
|
Does Http.Sys have any auto-flush behavior when buffering? E.g. if I enable buffering, write 5 bytes and then wait, will those bytes eventually be flushed on their own? Or only when I write more and fill the buffer? If not then this could break SignalR. Wasn't this flag already in use for WebSockets? |
davidni
force-pushed
the
davidni_kernelbuffering
branch
from
9732d75
to
67a8942
Compare
|
|
Surely seems that way, because I see bytes flowing to the client before the final Write happens on the server. Also, since this is still disabled by default, it wouldn't break any scenarios unless someone opts in for this. |
davidni
force-pushed
the
davidni_kernelbuffering
branch
from
67a8942
to
e2e5779
Compare
|
I meant does the buffer get auto-flushed if you pause writing in the middle? If not then we may need to make this a per-request feature so SignalR can disable it. |
|
Sorry this got put on hold so long. A change like this needs benchmarks, wider functional testing, etc. and we don't have bandwidth to follow through on those right now. Closing this for now and using https://github.com/aspnet/HttpSysServer/issues/41 to track future work here. |
This PR introduces a new setting
EnableKernelResponseBufferingthat causes theResponseStreamto pass theHTTP_SEND_RESPONSE_FLAG_BUFFER_DATAflag to Http.sys as appropriate when sending responses.Non-scientific benchmarks show dramatic perf wins when the response stream is written to in small amounts at a time (e.g. as done by aspnet/proxy), especially for networks with long round trip times.
After this change, I can get consistently more than 18 MB/s when serving static files through a proxy (similar to aspnet/proxy), hosted on an Azure A4 VM.