spacebudz / lucid

Lucid is a library, which allows you to create Cardano transactions and off-chain code for your Plutus contracts in JavaScript, Deno and Node.js.
https://lucid.spacebudz.io
MIT License
339 stars 139 forks source link

Stop Leaking Memory #234

Open yHSJ opened 11 months ago

yHSJ commented 11 months ago

At JPG Store, we have been dealing with memory leaks in our code that uses either Lucid or the CML. After some research, we discovered that the CML does a lot of clones to avoid issues with freed memory across the WASM boundary ( https://github.com/dcSpark/cardano-multiplatform-lib/issues/142#issuecomment-1281810732).

It turns out we were not the only ones to notice it, as you can see from this issue: https://github.com/spacebudz/lucid/issues/222

Any object that is returned from WASM to JavaScript seems to never get garbage collected. Thus, all objects returned from WASM to the JS runtime must be freed, including intermediary values when chaining function calls together. We experimented with the FinalizationRegistry API and introducing destructors to clean up memory, but this experiments proved to be unsuccessful. Even if they were successful, it would entirely depend on the user's runtime engine if it were impactful for them. That's why we settled on explicitly freeing memory through the new APIs we introduced.

This PR introduces proper memory management internally without introducing breaking changes to the API. It also exposes new methods on classes that include fields from the CML to free that memory, and new APIs to handle freeing memory on the user side (if they are using CML stuff directly).

AlexDochioiu commented 9 months ago

@yHSJ We use lucid extensively on vespr servers too and there's no memory leaks in it. I had a discussion with Ales a while ago when I thought I may have some leaks due to it (I was wrong).

Confirmed by Ales: Lucid is using a a version of CML built with weak refs support and does not need manual disposal for the objects (https://rustwasm.github.io/wasm-bindgen/reference/weak-references.html).

P.s. In my case, I was also using emurgo's CSL library on our servers, which was causing the memory leaks (in a few places where I missed the .free() call). I changed instead to not import the library published by emurgo and just use the CML built into lucid for my needs. Haven't had memory leaks since (for prob half a year now)

yHSJ commented 9 months ago

We use lucid extensively on vespr servers too and there's no memory leaks in it. I had a discussion with Ales a while ago when I thought I may have some leaks due to it (I was wrong).

Confirmed by Ales: Lucid is using a a version of CML built with weak refs support and does not need manual disposal for the objects (https://rustwasm.github.io/wasm-bindgen/reference/weak-references.html).

I think it is most likely that your servers are not running long enough to see a memory leak or you are not calling functions that are using the WASM bindings. Internal testing at JPG showed that Lucid was the cause of memory leaks and this PR directly fixed that issue. This has been corroborated by several other tests including by @dpbeaumont in https://github.com/spacebudz/lucid/issues/222 and several other developers privately.

klntsky commented 6 months ago

@yHSJ

We experimented with the FinalizationRegistry API and introducing destructors to clean up memory, but this experiments proved to be unsuccessful

What exactly didn't work?

We are using a vendored version[0] for quite a while, and it works well. Here[1] is our "GC" that uses FinalizationRegistry - and we have a test[2] that gives us some assurance.

[0] https://github.com/mlabs-haskell/cardano-serialization-lib-gc [1] https://github.com/mlabs-haskell/csl-gc-wrapper [2] https://github.com/Plutonomicon/cardano-transaction-lib/blob/develop/test/CslGc.js