-
Notifications
You must be signed in to change notification settings - Fork 16
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
Likely memory leak #49
Comments
+1, any info about this or whether it's going to be fixed? |
Got the same issue |
Depends on your use case, but for me all my assets run through webpack, so it was best to precompile all the brotli assets at max compression in the build, and serve them based on accepts headers. Then on-the-fly compression was not needed at all. |
Is there still a memory leak? Tag maintainer: @Alorel |
Has anyone double-checked that it isn't the caching behavior that's enabled
by default? Memory consumption graphs with a cache enabled can look just
like a memory leak
…On Thu, Jun 4, 2020, 08:53 Justsnoopy30 ***@***.***> wrote:
Is there still a memory leak? Tag maintainer: @Alorel
<https://github.com/Alorel>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#49 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAABZHMJMLIKTOCXULI7DXDRU67PHANCNFSM4KIKKBEQ>
.
|
The default cacheSize is 128mb, the graph shows it filling to my auto restart limit at 1gb, so I would say unlikely unless the cache sizing doesn't work. |
Is it easy to just completely disable the cache for your setup?
…On Thu, Jun 4, 2020, 09:23 Matthew Lein ***@***.***> wrote:
The default cacheSize is 128mb, the graph shows it filling to my auto
restart limit at 1gb, so I would say unlikely unless the cache sizing
doesn't work.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#49 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAABZHNUKG4QF23L24KDC23RU7DBBANCNFSM4KIKKBEQ>
.
|
I can't speak for anyone else, but for my setup, it was better to precompile brotli and gzip in my webpack build and just serve it based on headers. Then shrink ray was not needed. |
We used this dependency for compressing html on demand (generated from a react application) and I can confirm that it has a memory leak (using the latest versions of the dependencies on NodeJS v12.20). We saw the exact same graph as shown in the initial screenshot. After the deployment using compression middleware it is gone. As a positive side effect our CPU consumption is also down by at least 50%. User server pre-renders a react application server side. |
Brotli support seems to be one of the main benefits of shrink-ray-current compared to the compression library. If anyone else is looking for dynamic brotli compression of Express responses but are worried about issues in shrink-ray-current issue tracker, compression-next seems to be an npm publish of the most promising PR for adding brotli support in the main compression library. In any case, it seems the only realistic options right now for dynamic brotli compression of Express responses are shrink-ray-current and compression-next. |
Just did a release that swapped compression for
"shrink-ray-current": "4.1.2",
and this is the resulting memory (auto restart on a max memory limit drops it down)The text was updated successfully, but these errors were encountered: