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

Some memory leaks here and there due to cardano serialization lib #272

Closed
AlexDochioiu opened this issue Jun 7, 2023 · 8 comments · Fixed by #273
Closed

Some memory leaks here and there due to cardano serialization lib #272

AlexDochioiu opened this issue Jun 7, 2023 · 8 comments · Fixed by #273

Comments

@AlexDochioiu
Copy link

It seems that with CSL, every single object is always copied / returned as a copy and should be freed. This means that you cannot chain calls without causing a leak instantaneously.

Looking at helpers file, one such example is:

const stakeKeyHash = addressSpecificStakeKey.to_raw_key().hash();

The result of to_raw_key() is never stored in a variable and freed, hence it will leak memory endlessly.

Those should be split into

const stakeRawKey = addressSpecificStakeKey.to_raw_key();
const stakeKeyHash = stakeRawKey.hash();
...
stakeRawKey.free();
stakeKeyHash.free();

to avoid memory leaks.

@slowbackspace
Copy link
Contributor

@AlexDochioiu great catch! I've opened a PR with a fix. I'll run some tests to confirm that it indeed resolved the memory leak. Hopefully I didn't miss anything :)

#273

@AlexDochioiu
Copy link
Author

That was quick 😂 . I'm pretty much looking at all our usage (directly/indirectly) of CSL because our server is leaking 1GB per day because of it and going up 😬.
P.S. The issue is not actually from your lib cause I'm not even using those helpers but I thought it's worth mentioning.

@vladimirvolek
Copy link
Member

@AlexDochioiu Maybe you can try this? Emurgo/cardano-serialization-lib#542 (comment)

@slowbackspace
Copy link
Contributor

slowbackspace commented Jun 7, 2023

That was quick 😂 . I'm pretty much looking at all our usage (directly/indirectly) of CSL because our server is leaking 1GB per day because of it and going up 😬. P.S. The issue is not actually from your lib cause I'm not even using those helpers but I thought it's worth mentioning.

We are also experiencing memory leaks in some of our projects which, I assume, is because of CSL (called from deriveAddress). But for us the leak is manageable so it doesn't bother us that much.

I ran deriveAddress approx. 200k times (for different address indexes) and it seems that process.memoryUsage() reports about 50MB increase in external where (I assume) should be all WASM bloat. This memory usage is basically same in master branch and PR's branch so I'm not sure how much did the additional frees() improved things, but it certainly does make sense.

What are you using to track down the memory leak?

@AlexDochioiu
Copy link
Author

Took heap snapshots at fixed intervals and checked/compared them.

@slowbackspace
Copy link
Contributor

Took heap snapshots at fixed intervals and checked/compared them.

Oh interesting! I didn't expect it would affect heap since in our projects we are mostly seeing leaks in external memory, but the fix does save about 20MB of heap mem (per 200k derivations). Thank you!

@AlexDochioiu
Copy link
Author

@AlexDochioiu Maybe you can try this? Emurgo/cardano-serialization-lib#542 (comment)

Unfortunately my main expertise is not exactly rust/wasm/nodejs and I basically ran into issues trying to recompile CSL myself. Also decided it's not worth the effort to debug it any further as I'm almost fully rolling it out of our codebase in favor of Lucid (which uses CML built with that flag enabled ; can already confirm that there's no signs of any leaks coming from lucid's usage of CML).

@vladimirvolek
Copy link
Member

@AlexDochioiu Maybe you can try this? Emurgo/cardano-serialization-lib#542 (comment)

Unfortunately my main expertise is not exactly rust/wasm/nodejs and I basically ran into issues trying to recompile CSL myself. Also decided it's not worth the effort to debug it any further as I'm almost fully rolling it out of our codebase in favor of Lucid (which uses CML built with that flag enabled ; can already confirm that there's no signs of any leaks coming from lucid's usage of CML).

Thank you for the info ❤️

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

Successfully merging a pull request may close this issue.

3 participants