Reduce lifetime of header values

[benaadams]

For common header values (non-dictionary backed ones)

• IHttpComponentFactory controls header and stream creation, disposal
• Headers are attached at request start and detached at request end
• Streams are attached at message body start and detached at request end
• HttpComponentFactory implementation pools the headers, and streams
  if kestrel.maxPooledStreams > 0 || kestrel.maxPooledHeaders > 0
• Request header values are nulled before detach (User input could be of indeterminate length)
• If connection errors in-use headers & streams are not returned to pool

Faster cycling of header objects should cause the Response header values to remain in Gen0 as they are likely to be reset to a new value on each request.

Request values are reset if they have been set

Streams are uninitialized and reinitialized with frame reference so as not to prolong its lifetime.

Also doesn't hold onto header collections or stream objects while the Frame is in a keep-alive wait.

Resolves #607
Resolves #628
Resolves #629 when set > 0

Before:

With this change, setting

{
  "kestrel.maxPooledStreams": 128,
  "kestrel.maxPooledHeaders": 128
}

After memory is down by 1.4GB, headers have moved out of the top spots

And instead appear quite far down

[muratg]

nit: this is ordered differently than Pause and Stop :) (Response/Request here, vs Request/Response on the others.)

[halter73]

Is there any evidence this is faster than simply resetting all the fields without checking which bits are set like we did before?

[benaadams]

Could do it in one statement by putting them in a struct and doing _headers = default(Headers)

[Tratcher]

Hey @benaadams, while you're at it... #628

[halter73]

I'm not sure this pattern is really achieving anything.

[benaadams]

Is to detect double call; also to drop references that have been returned to pool

[benaadams]

This shouldn't be in this set

[benaadams]

Need to look at clearing methods

Is there any evidence this is faster than simply resetting all the fields without checking which bits are set like we did before?

[benaadams]

Update description with measurements

[benaadams]

Added fast header clear.

Note: Also you will need to switch on some pooling for high perf as headers now are created and dropped per request rather than per connection.

[benaadams]

So with the inner struct clearing out all the header strings in ClearFast isn't much il

  IL_0009:  ldflda     valuetype HeaderReferences FrameRequestHeaders::_headers
  IL_000e:  initobj HeaderReferences

Full function

.method family hidebysig virtual instance void 
        ClearFast() cil managed
{
  // Code size       37 (0x25)
  .maxstack  8
  IL_0000:  ldarg.0
  IL_0001:  ldc.i4.0
  IL_0002:  conv.i8
  IL_0003:  stfld      int64 Microsoft.AspNetCore.Server.Kestrel.Http.FrameRequestHeaders::_bits
  IL_0008:  ldarg.0
  IL_0009:  ldflda     valuetype Microsoft.AspNetCore.Server.Kestrel.Http.FrameRequestHeaders/HeaderReferences Microsoft.AspNetCore.Server.Kestrel.Http.FrameRequestHeaders::_headers
  IL_000e:  initobj    Microsoft.AspNetCore.Server.Kestrel.Http.FrameRequestHeaders/HeaderReferences
  IL_0014:  ldarg.0
  IL_0015:  ldfld      class [System.Collections]System.Collections.Generic.Dictionary`2<string,valuetype [Microsoft.Extensions.Primitives]Microsoft.Extensions.Primitives.StringValues> Microsoft.AspNetCore.Server.Kestrel.Http.FrameHeaders::MaybeUnknown
  IL_001a:  dup
  IL_001b:  brtrue.s   IL_001f
  IL_001d:  pop
  IL_001e:  ret
  IL_001f:  call       instance void class [System.Collections]System.Collections.Generic.Dictionary`2<string,valuetype [Microsoft.Extensions.Primitives]Microsoft.Extensions.Primitives.StringValues>::Clear()
  IL_0024:  ret
} // end of method FrameRequestHeaders::ClearFast

[halter73]

🚢

[halter73]

Don't return this. I don't see you using this anywhere anyway.

[halter73]

On second thought, don't worry about this. I changed it while addressing my own PR feedback readying a merge.