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

call for encode br support for brotli compression #5194

Closed
orca-zhang opened this issue Nov 10, 2022 · 13 comments
Closed

call for encode br support for brotli compression #5194

orca-zhang opened this issue Nov 10, 2022 · 13 comments
Labels
duplicate 🖇️ This issue or pull request already exists

Comments

@orca-zhang
Copy link

orca-zhang commented Nov 10, 2022

I found brotli is more widely used/supported on mobile browsers than gzip these days.

I think, it's about ~23.33%(96.55%-73.22%) delta that is announced.

https://caniuse.com/brotli

https://caniuse.com/mdn-api_compressionstream_compressionstream_gzip

BTW, thanks a ton for your great Caddy, I really like it.

@orca-zhang orca-zhang changed the title call for encode brotli support call for encode br support for brotli compression Nov 10, 2022
@mholt
Copy link
Member

mholt commented Nov 10, 2022

Glad you like Caddy!

Unfortunately, performance of compressing brotli on-the-fly is unacceptable with current implementations. (My understanding is that the brotli format itself is more expensive to encode than decode, so it's possible that it may never be within tolerances.) If it can be made fast enough (with pure Go and/or assembly) then we are open to doing it.

Kind of a duplicate of #3704

Related: #3692

We encourage clients to support Zstandard for more modern on-the-fly compression.

@mholt mholt closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2022
@mholt mholt added the duplicate 🖇️ This issue or pull request already exists label Nov 10, 2022
@tasiotas
Copy link

tasiotas commented Mar 1, 2023

Hi @mholt

I totally see your point of Brotli being too slow for on the fly compression, but what about case where Caddy request is being cached? Not having Brotli at all is quite limiting. I'm serving content from 3rd party softwre that does not give me option to precompress assets. Caddy would be perfect place to handle it, and use caching down the pipe.

Thanks

@francislavoie
Copy link
Member

There exists a plugin to provide brotli support. You can use that.

But because of the performance concerns, it does not make sense to include in Caddy's standard distribution because users will use it without understanding the performance implications then complain to us when they have very high CPU usage. So we're careful about what we include by default.

@tasiotas
Copy link

tasiotas commented Mar 1, 2023

oh good to hear, thanks!

@mholt
Copy link
Member

mholt commented Mar 1, 2023

If you can find us an optimized implementation in Go, we will happily use it. But current implementations grind servers to a halt. Brotli is just hard to encode efficiently.

@sowinski
Copy link

sowinski commented Dec 4, 2023

Hi, why can you not call from caddy a c library which is doing this faster?

@mohammed90
Copy link
Member

mohammed90 commented Dec 4, 2023

Hi, why can you not call from caddy a c library which is doing this faster?

The top reasons are:

  • portability: cross-compiling a project with CGO is not fun
  • performance: CGO calls are not free, and the compromise in performance has to be truly worth it

Cockroach Labs have a good blogpost about the overhead of using CGO: The cost and complexity of Cgo

@mholt
Copy link
Member

mholt commented Dec 4, 2023

Also memory safety. No good if a file can cause a C library to pop out of its memory bounds.

@dunglas
Copy link
Collaborator

dunglas commented Jan 17, 2024

For the record, I created a module that relies on the official C implementation (and the Google Go module): https://github.com/dunglas/caddy-cbrotli

@sowinski
Copy link

Is this implementation fast enough to run it for dynamic created content or is it only good for static files?

@dunglas
Copy link
Collaborator

dunglas commented Jan 18, 2024

@sowinski according to my tests it is good enough dynamic content.

@sowinski
Copy link

would be nice if this feature would fine the way to the next caddy version

@mohammed90
Copy link
Member

would be nice if this feature would fine the way to the next caddy version

Kevin's plugin still uses C (CGO). There's near zero chance we're admitting C dependency into Caddy. Plugins can be developed and built for users who desire to accept the risk and complexity of having C in the stack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate 🖇️ This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

7 participants