nytimes / gziphandler

Go middleware to gzip HTTP responses
https://godoc.org/github.com/NYTimes/gziphandler
Apache License 2.0
871 stars 128 forks source link

Swappable gzip implementation? #94

Open whs opened 4 years ago

whs commented 4 years ago

Hi!

We at @wongnai have forked gziphandler internally to add swappable gzip implementation. In production, we swap compress/gzip with our fork of yasushi-saito/cloudflare-zlib which result in 43% less CPU percentage used in the middleware.

We haven't open sourced anything in this project yet, as they require extensive modification to all projects to make it work. I'd like to check with upstream first whether this is something you'll be willing to merge before starting to work on open sourcing it (eg. unfork the go module name).


The changes we made are:

type GzipWriter interface {
    Close() error
    Flush() error
    Write(p []byte) (int, error)
}

For forked cloudflare-zlib and its integration with gziphandler we may open source it later after the PR made here is merged. We removed its built-in zlib code and just simply link with installed library.

CAFxX commented 4 years ago

I'm also looking at this, and would love to have a swappable gzip implementation - or even just a faster builtin gzip implementation.

Profiling shows that gzip compression accounts for ~20% of the CPU cycles on our backends.

harrisonhjones commented 4 years ago

Implementations are swapped by build tag. The default build still use compress/gzip to avoid extra non-Go dependency.

Is this needed? What if gziphandler was updated to take an (optional) GzipWriter and then users could customize it was needed (or not)? Or maybe I'm misunderstanding what you are proposing.

whs commented 4 years ago

Thanks for the suggestion, I agree that swappable interface is a better way to write it.

I've opened #106 to start open sourcing this

dolmen commented 3 years ago

I like your refactor in #106 and I think it should be merged.

However your alternate implementation of GzipWriter must not be in the same Go module (it might even be in another repo) for the following reasons:

whs commented 3 years ago

Good to see this thread still moving, although upstream still doesn't have plan to merge.

Haven't open source our gziphandler interface yet (maybe after it was merged). Our cloudflare-zlib was open sourced after this thread at https://github.com/wongnai/cloudflare-zlib . The major changes was to remove building of zlib with CGO (some glue C code remains) and instead dynamically link with it.

klauspost commented 3 years ago

https://github.com/klauspost/compress/tree/master/gzhttp#gzip-middleware is an updated and cleaned up fork.

CAFxX commented 3 years ago

Just dropping by to mention that I also forked this library and added support for pluggable implementations (in addition to other features, like support for encodings beyond gzip, like zstandard and brotli): https://github.com/CAFxX/httpcompression