search

aspnet / BasicMiddleware

[Archived] Basic middleware components for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
169 stars 84 forks source link

ResponseCompressionMiddleware causes overhead even if content type is not compressible #142

Closed pakrym closed 8 years ago

pakrym commented 8 years ago

It calls GetCompressionProvider for every request before checking content type.

@Tratcher

Tratcher commented 8 years ago

It first checks if the client accepts compression. If not, it avoids the overhead of wiring up a special response stream.

The alternative would be to delay both checks until the first write, but you'd always have to wire up a special response stream. There's overhead either way, pick your poison.

pakrym commented 8 years ago

Don't we already have custom BodyWrapperStream?

Tratcher commented 8 years ago

Not for this code path: https://github.com/aspnet/BasicMiddleware/blob/dev/src/Microsoft.AspNetCore.ResponseCompression/ResponseCompressionMiddleware.cs#L60-L66

pakrym commented 8 years ago

Most browsers send Accept-Encoding header with compression we support.

Tratcher commented 8 years ago

True, but what about all the WebApi clients?

pakrym commented 8 years ago

Lest phrase it the other way, in 90% cases if Accept-Encoding is not empty it would contain compression we support.

Tratcher commented 8 years ago

Probably true, GZip is the standard compression on the internet. But your assertion leaves out the cases where the Accept-Encoding header is not present, e.g. many WebApi clients do not include it. I wonder where we could get metrics on that.

These are the questions that need to be measured: 1) What percentage of responses match the default content-type list for compression? 2) What's the cost overhead of creating the response stream wrapper? 3) What percentage of request include an Accept-Encoding header? 4) What's the cost of inspecting the Accept-Encoding header on a request when it's a common value like gzip, deflate. We already know this can be optimized with fast-paths for common headers, right now it always checks for corner cases like q-values.

pakrym commented 8 years ago

If Accept-Encoding is not present we don't need to do anything, middlware just short circuits.

So the logic should be:

  1. If no Accept-Encoding header exist - exit.
  2. If Accept-Encoding present wrap the stream.
  3. On first write if Content-Type is compressible try searching for provider.