search

cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.07k stars 397 forks source link

[jss-cache] Memory leak on server side #1022

Closed anechunaev closed 5 years ago

anechunaev commented 5 years ago

Hi. In my project I rely on dynamic styles a lot. I didn't found out yet why, but on server side Cache object in the plugin jss-cache saves dynamic styles between requests. It causes heap size bloating.

Example project: https://github.com/bungu/jss-cache-memory-leak-example

Memory usage in example project with jss-cache: 2019-02-06 13 45 55

Memory usage without jss-cache: 2019-02-06 13 55 52

In real app it increases more dramatically. I tried to swap Cache object with WeakMap like in the alpha version, but it still leaks.

jss: v9.8.7 jss-cache: v3.0.0 react-jss: v8.6.1

kof commented 5 years ago

You probably run into the caveats that are part of the description https://cssinjs.org/jss-plugin-cache?v=v10.0.0-alpha.9#caveats

kof commented 5 years ago

I do want to have a better caching implementation that could be evtl be built in by default, but there are certain issues to overcome.

HenriBeck commented 5 years ago

Have you also tried it with the v10 alpha?

HenriBeck commented 5 years ago

Also, do you mean with dynamic styles themed styles or function values? Because for function values we don't add the rules to the cache.

My guess is that because you get a new object from the themed styles function for every request we add it to the cache, and it's not being cleared for some reason.

anechunaev commented 5 years ago

I think this is my case:

Don't use it if you generate a huge amount of different rules. E.g., if you generate for every request or every user, different styles. The cache memory footprint will grow proportionally to the number of unique styles.

But I only use themed styles, they are not unique for different users or requests. Maybe themed styles are "unique" because themed styles function return new object every call and we have no way to compare such objects. Probably docs need to be updated?

I have not tried alpha version, but I edited jss-cache source code the same way as from v10 alpha. There is a leak even with WeakMap. I also tried to run GC manually after each render, it didn't work.

I think we can close this issue since it's covered in docs.

HenriBeck commented 5 years ago

Yes, the themed styles are the problem. Even if they are being called with the same theme, they will return a new object. This leads to the memory leak. I'm not sure how we can avoid this, as themed styles shouldn't be cached on the server in my opinion.

We should update the docs to use the cache plugin with caution, especially on the server with themed styles.

kof commented 5 years ago

There is already a caveats section in the plugin documentation, we should just add another point regardig themed styles. Also evtl. we could find a way to make it work where it can work reliably, because currently its just too dangerous because it can shoot you into the foot any time.