-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
π [Bug]: β500ms Performance penalty using compression on and 'br' in 'Accept-Encoding' #2722
Comments
There is actually nothing in the last version that can trigger this |
I will test it with the last previous 3 versions and report back. |
Ok thx |
To diagnose where the issue is you can add package main
import (
"github.com/gofiber/fiber/v2"
"github.com/gofiber/fiber/v2/middleware/compress"
"github.com/gofiber/fiber/v2/middleware/logger"
)
func main() {
app := fiber.New()
app.Use("/", logger.New(logger.ConfigDefault))
app.Use("/", compress.New(compress.Config{Level: 2}))
app.Static("/", "")
app.Listen(":3030")
} You will see logs that show approximately how long fiber took to process the request.
So if it's significantly different between versions of fiber, there's a problem |
I always use the middleware logger. But this in this log it was not showing it - I can state that directly. Ofc there could be a discrepancy when using Nginx and Fiber, but I will test first and then provide all the info. |
Ok, I just performed some tests:
and could not reproduce the bug. But I just noticed that also added a custom function to the engine, to be able to render HTML as Here the added custom-function, which I added to the engine:
I will try some more tests and update the docker-images afterwards - so the links then will reflect the updated state |
@the-hotmann so it cannot be reproduced? can we then close this issue until it is reproducible and then reopen it if necessary? |
I woul like to keep it open for some more hours, as I will re-try to reproduce it. Thanks |
ok |
I did some more tests and could reproduce the error again. Feel free to download all 5 versions/tags from this docker-image: https://hub.docker.com/r/h0tmann/go_fiber_test
Now use Nginx/NPM to point a domain to each of the containers. You will end up with two possible URLs to call the very same container:
DO NOT TEST BEHIND CLOUDFLARE! My public Tests are hosted like this:
My internal Tests are tested like this:
I am more then happy to provide you guys with the new URLs, apparently they point to IPs which I dont want to share publicly. So feel free to contact me privately or setup your own testcase according to the tutorial above. Also I tested on the Versions:
It happens on ALL of them. @nickajacks1 thanks for looking into it. I personally have some guesses, but I will just dump on every request the request header and print it to the HTML, then we can see what is the difference. Will rewrite my testcases and rebuild.
|
can you rule out that it's not the infrastructure you built around fiber? |
can you find out the version where it first appeared? |
Indeed I can not, I have no proof it is fiber. But it completely disappears when turning off:
Still could be nginx, but again, if behind nginx AND Cloudflare it does not appear. Currently a blackbox.
I will try to find out. |
Some additional Info: For the first time (I even recorded it) the app (
Here the video: fiber_compression_test.MP4P.S.: it crashed because it was OOM. When the thing happens (the high TTFBs) every reload consumes abour 30MB RAM, which will not be cleared by the GC for a long time (if ever). |
Tested down to Maybe it is not the app itself, but the template package? Will try to downgrade this.
|
I added the functionality to print out the request header. Fast/Direct:
Slow/Nginx:
Maybe it does have something to do with the discrepancy of the Also the:
Is strange, it should actually keep the connection opened. This is the current full fibver config:
Some parts of the |
With Cloudflare the problem does not occur and The request Headers through Cloudflare look like this:
Here again is no
|
Can you try the same or similar without fiber? Have you also activated compression in nginx for the routes? Then I would deactivate one of the two Please try to narrow it down further |
I have to decline. Setup fiber with
start the App Now run these two curl requests directly and locally against your application: 1.)
2.) (Note: with
For me the results are: 1.)
2.) (Note: with
I have run this multiple times. It all times gives me the same result. My analysis: |
2
on v2.51.0
Ok that's interesting Maybe brotli just needs more time By the way, we use a fasthttp middleware in our middleware for handling the compression, which internal uses a additional lib for it So we need to find out which of these components is affected |
When I use br encoding on a 963KB file with level 2 compression on a bare fiber server on localhost, I get these results:
The whole request takes forever, but TTFB isn't as bad. Deflate and gzip are also slowish but still much faster (~150ms real time) But if I use level 1 compression with brotli, it's a lot faster
|
I am there :)
The fastest response from a fiber server I get, when I use
Here my results with
|
Via Discord the problem could be locally reproducts, but it just seems that Compression-Level 2 (brotli lvl9 LINK) ist just very CPU intense and takes much longer than other compression techniques on theirhighest level. So even if a single request took about 0.5s, this is to be expected when using brotli on its highest level, combined with some 100kb of text. @ReneWerner87 thank you for the clearification! P.S.: just don't use compression level 2 in the |
Bug Description
Usually I by default set compression to
2
but today I noticed a problem when on fiberv2.51.0
.This is the config I use:
I tested a bit and came to this conclusion:
a direct call will always be fast, but as soon as I access a fiber 2.51.0 pages I see TTFBs of at least 400-600ms.
This penalty just appears to be happening on the HTML part, not any static assets.
Turning off compression solves the problem entirely.
So I had multiple thoughts:
How to Reproduce
Actually already described above
Expected Behavior
New features can have a performance penalty, but 400-600ms is not acceptable I guess?
Fiber Version
v2.51.0
Code Snippet (optional)
No response
Checklist:
The text was updated successfully, but these errors were encountered: