Expose Content-Length in IHttpResponseFeature

[benaadams]

At the moment accessing content-length goes to string and back through parsing and the header collection. It is a long? on Response.

The string transitions are alloc heavy for 2000 requests at 108k meaning 54 Meg per 1M requests:

What I'd much prefer is to have the long? directly so I can output that straight to bytes without going via string or allocating.

[Tratcher]

Is the cost you're measuring for long -> string -> ascii bytes, and you're trying to shorten it to long -> ascii bytes?

Background: System.Web has had no end of problems special casing certain headers to bypass the header collection. Response cookies are a particularly bad example where it keeps a separate collection for them and only adds them to the headers at the end of the response. This causes conflicts for anyone using the headers directly. We don't want to repeat this mistake so we've kept the headers collection as the source of truth for all headers.

[benaadams]

Trying to shorten it to long -> pre allocated byte buffer; so zero alloc.

Am ok with the other headers, as they come with a user generated cost, and where the user has already created the string; but this is generally always present except if chunked and is a hidden library cost that cannot be worked around.

[benaadams]

For comparison its 30% of the alloc cost of DI: aspnet/DependencyInjection#291

[Tratcher]

StaticFiles does set it but most of our other components don't, they rely on the server to chunk or to buffer and set it for them. In those cases the server can take shortcuts internally.

I can't think of any of our other libraries that set it by default.

[rynowak]

MVC does in some extremely obscure scenarios, but not anything @benaadams or the average user is likely to have encountered.

[benaadams]

With the current set up this is ok:

httpContext.Response.Headers["Content-Length"] = "fred";

But using the strongly typed ContentLength on HttpResponse as used by plaintext benchmark is an anti-pattern:

httpContext.Response.ContentLength = _helloWorldPayload.Length;

Which feels a bit wrong.

This is because it goes ContentLength->new HeaderDictionary->ParsingHelpers.SetContentLength->value.ToString()->IDictionary[key]=

If IHttpResponseFeature implemented ContentLength it could chose to go via the headers; but it wouldn't have to.

Also the servers response.write can't assume the result is complete until the invoked closure is complete.

[davidfowl]

The content length being set has to appear in the headers dictionary. It could be lazy be we want a single source of truth. Managing separate ways to set data is a PITA (we know from experience)

[benaadams]

Managing separate ways to set data is a PITA (we know from experience)

There are already 2 ways; strongly typed on Request route and weakly typed via headers. They both resolve to headers.

You could change this so setting via headers goes to IHttpResponseFeature.ContentLength with a type check on setting that throws an exception if it can't be set to a long or null. Since there is a level of indirection on setting headers this would be possible.

For output it would follow a similar path to StatusCode and ReasonPhrase?

[benaadams]

Also it means when deciding to chunk the check is: ContentLength.HasValue vs HeadersDictionary.Contains("Content-Length")+ignore-case

[benaadams]

Content-Length can only be a numeric >= 0 http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.13

but you can set it to a whole bunch of weird via the headers, including multiple values; which if you try to read back via ContentLength will just say its not set

I think that's all I've got; will leave this bone alone now :)

[benaadams]

To resolve this issue you'd need to convert it from the header back to a long aspnet/KestrelHttpServer#175

Though kestrel does do the work of re-parsing from the header back to an int perhaps for this reason: https://github.com/aspnet/KestrelHttpServer/blob/c50aec17292b9f6d444da5f11637fd3c9146146d/src/Microsoft.AspNet.Server.Kestrel/Http/MessageBody.cs#L54

[benaadams]

Final thing; have done some improvements to server and now find myself in the particular situation of setting content length being the big allocator in the plaintext middleware (2k requests):

Ok, now am done...

[muratg]

Moving this to Backlog as we will be in RC2 ask mode very soon. If you feel strongly about this issue, please ping me.

[benaadams]

I feel strongly about it; but am ok with it being a post RTM discussion (although it would be a breaking change)

[benaadams]

To clarify: Is breaking change for servers, not for end users as the attribute is already exposed - its just how its implemented that's emulated.

[benaadams]

Did some measurements; the difference between these two lines

response.ContentLength = 13;

response.Headers["Content-Length"] = "13";

Is as follows

         Method |                 Type |        Mean |    StdDev |           RPS | Allocated |
--------------- |--------------------- |------------ |---------- |-------------- |---------- |
 BenchmarkAsync | ContentLengthNumeric | 115.8341 ns | 1.4149 ns |  8,633,038.71 |      32 B |
 BenchmarkAsync |  ContentLengthString |  80.2629 ns | 1.1965 ns | 12,459,057.14 |       0 B |

[benaadams]

Benchmarks aspnet/KestrelHttpServer#1256

[Tratcher]

2.0 Proposal:
Put long ContentLength { get; set; } on IHeaderDictionary, not IHttpResponseFeature. The IHeaderDictionary implementation can maintain it's own consistency and optimize any lazy conversion between strings and longs as needed. ContentLength is an especially good candidate for IHeaderDictionary because it applies equally to requests and responses.

Are there any other headers that would benefit from direct IHeaderDictionary properties?

[davidfowl]

The Date header?

[benaadams]

The Date header?

Any you'd want to though? People wouldn't normally read the date request header, and shouldn't be encouraged to write the date response header 😉

[benaadams]

Thank you for resolving this 😁