Use Inheritance instead of Composition to Reduce allocation in Request Decompression Middleware

[Kahbazi]

I've opened this issue to discuss the idea that I've mentioned in here and here now that Request Decompression Middleware is merged.

At the moment IDecompressionProvider is creating a decompression stream and the middleware creates a SizeLimitedStream which wraps the other one. We could reduce this two streams into one if the IDecompressionProvider returns a stream that also handle size limit check, and the middleware only create SizeLimitedStream if the returned stream from the IDecompressionProvider is not marked with IDontNeedWrapper interface.

+internal interface IDontNeedWrapper {}
+internal sealed class GZipRequestDecompressionBody : GZipStream, IDontNeedWrapper {}
+internal sealed class DeflateRequestDecompressionBody : DeflateStream , IDontNeedWrapper {}
+internal sealed class BrotliRequestDecompressionBody : BrotliStream, IDontNeedWrapper {}

The new types for this change are all private, so there's no new public types. By doing this we would reduce one allocation per requests when request body is compressed by GZip, Deflate or Brotli.

[adityamandaleeka]

Triage: we'd need to see perf numbers to decide whether the impact of this is worth the complexity. A small improvement in this decompression scenario may not be worth it...

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.

[david-acker]

I made these changes locally and there only seems to be an allocation-related improvement for compressed requests when IRequestSizeLimitMetadata exists (reduced from 3 KB to 2 KB). Apart from that, it doesn't look like much else changed, although I'm not too familiar with interpreting these benchmarks.

Benchmark Results

I also noticed a few complications when implementing and testing this:

1. The BrotliStream class is sealed, so we'll only be able to create new types for Deflate and GZip.
2. The CopyTo and CopyToAsync methods on the GZipStream pass their operations to a DeflateStream that's used internally which circumvented the size limit check I added and caused some test failures. I not sure if this was to be expected, but it doesn't seem like adding the size limit check will be as straight forward as overriding the Read and ReadAsync methods like it was for the original SizeLimitedStream.