Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: turn off gzip compression #65

Open
wvh opened this issue Jan 18, 2019 · 3 comments
Open

Feature: turn off gzip compression #65

wvh opened this issue Jan 18, 2019 · 3 comments

Comments

@wvh
Copy link

wvh commented Jan 18, 2019

Hello,

There doesn't seem to be an option to turn off automatic compression of included files. The findAndWriteFiles uses size comparison passed from writeCompressedFileInfo to detect whether it would be better to serve the resource as-is or compressed with gzip. This is a cool feature, but there is no way to turn it off when disabling compression application-wide for security or performance reasons.

I benchmarked HTTP responses – a byte array resource at ~83000 req/sec, an uncompressed http.FileSystem resource at ~78000 req/sec, and a gzip'ed resource that has to be decompressed on the fly at ~20000 req/sec, so the performance is about 1/4 for small ~1024 byte resources versus non-compressed byte arrays. Serving gzip'ed content without actually handling compression headers thus comes at a noticeable performance cost.

It would be nice if there was a way to set a configuration option in Options and pass this to the file system walking function to disable all compression.

@dmitshur
Copy link
Member

dmitshur commented Jan 19, 2019

Hi, thanks for raising an issue about this. I have some questions to understand this better.

There doesn't seem to be an option to turn off automatic compression of included files.

This is currently true.

gzip'ed resource that has to be decompressed on the fly

Decompressed by what? The browser, or your Go program?

When you performed the benchmark, did you use httpgzip.FileServer which uses the GzipByter interface to avoid unnecessarily decompressing gzip-compressed bytes from vfsgen output when serving HTTP clients that accept gzip compression (most modern HTTP clients)?

Serving gzip'ed content without actually handling compression headers thus comes at a noticeable performance cost.

Can you please clarify what you mean by "without actually handling compression headers"? I'm not sure which headers you're referring to.

It would be nice if there was a way to set a configuration option in Options and pass this to the file system walking function to disable all compression.

In general, I prefer to expose as few options as viable. There's a high bar in order to add one, but I will consider it if there's sufficient justification for it.

@wvh
Copy link
Author

wvh commented Jan 21, 2019

When you performed the benchmark, did you use httpgzip.FileServer which uses the GzipByter interface to avoid unnecessarily decompressing gzip-compressed bytes from vfsgen output when serving HTTP clients that accept gzip compression (most modern HTTP clients)?

[...]

Can you please clarify what you mean by "without actually handling compression headers"? I'm not sure which headers you're referring to.

I was referring to when one serves those compressed static resources without using your GzipByter interface and package, or otherwise doesn't manually read and set compression related headers; in other words, when someone uses vfsgen by itself in a default Go http setup.

Benchmarking software, programmatic API clients, curl and other HTTP clients apart from end-user browser software typically do not send headers to indicate they want or can handle compressed content.

It would be nice if there was a way to set a configuration option in Options and pass this to the file system walking function to disable all compression.

In general, I prefer to expose as few options as viable. There's a high bar in order to add one, but I will consider it if there's sufficient justification for it.

I understand that. There are some use cases where the static files are few, very small or perhaps simple (http/template) templates where the gain of compression is low or non-existing; and use cases where the HTTP clients are not modern end-user browsers and those clients might not indicate or support gzip encoding.

But even if you want to prioritise the typical end-user browser setup – which is certainly a valid approach – I think it would be a good idea to include a concrete example of serving the content with the httpgzip.GzipByter interface and/or a warning that vfsgen will decompress on-the-fly otherwise, because it is not immediately clear that that is what happens when a user of this package would just plug it into a typical Go http package handler without actually using the provided interface or dealing with Accept-Encoding and Content-Encoding headers.

Thanks for your response!

@ncruces
Copy link

ncruces commented Feb 1, 2019

If you're going to add an option to the interface...

May I suggest that instead of a flag to turn on/off compression you accept an int, and use the constants from here (better documented here)?

In fact I was about to suggest that this package always used BestCompression (the only downside I can see would be go generate becoming slower, other than that performance should increase).

I wouldn't mind doing the legwork if it's something you'd be interested in merging. Right now I have my own fork with gzip.NewWriterLevel(sw, gzip.BestCompression) inserted here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants