dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.4k stars 10k forks source link

Implement SendFileAsync feature #3807

Open shirhatti opened 6 years ago

shirhatti commented 6 years ago

For In-Proc

https://github.com/dotnet/aspnetcore/blob/d804e5d6fd6158c3ebc96e1b87788c9b9fc08d36/src/Http/Http.Features/src/IHttpResponseBodyFeature.cs#L44

muratg commented 6 years ago

@shirhatti Trying to focus 2.2 a bit more. Tentatively putting this in 3.0.

davidfowl commented 3 years ago

I would love to see some basic static files tests automated in the lab before we make this change.

@sebastienros I know we have a benchmark, its just a matter of adding it to the list of benchmarks right?

sebastienros commented 3 years ago

We have scenarios. My issue is what we try to measure. What does it mean to have a good static file perf?

davidfowl commented 3 years ago

You're right. I don't know if hammering the server with 1M RPS makes sense for static files. Typical usage might be an initial request for a bunch of resources in parallel. After that they're cached by the client.

We could grab something like amazon or facebook's client side resources and then send bursts of concurrent connections to access the resources. Large number of connections, small RPS.

Any thoughts @Tratcher @halter73 @BrennanConroy

halter73 commented 3 years ago

I think bursts of concurrent connections leading to a large number of connections and small RPS will still quickly become bandwidth constrained after a small initial ramp up. If we still use RPS as the metric for this kind test, it will probably benefit algorithms with less initial batching because smaller initial batches leads to less latency and a faster ramp up to saturating the NICs.

Another metric we should keep in mind in addition to the list @sebastienros listed is fairness. We wouldn't want to switch to an algorithm that used less CPU by working on larger batches at the cost of unfairly slowing down unlucky connections. This is especially true for static file serving scenarios with a lot of connections where it seems unlikely the server will be CPU constrained.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.