search

andywer / leakage

🐛 Memory leak testing for node.
1.58k stars 52 forks source link

How many iterations to do? #20

Closed mishrsud closed 7 years ago

mishrsud commented 7 years ago

Hello Maintainers, Please accept a hat-tip, love what I see thus far in the project. I have a question/comment regarding the wording here:

Nevertheless you can try to reduce the iteration count (the first parameter passed to iterate()). If you use 100 iterations as in the sample, try 30 iterations instead.

From past experience, generally profiling tests encourage having a sufficiently large iteration count, so this suggestion sounds a tad counter-intuitive. A lower iteration count may produce false-positives (unless there is a glaringly obvious leak that is caught easily). I believe this phrase should be removed. What are your thoughts?

andywer commented 7 years ago

Hey @mishrsud. Thanks for your feedback! :)

Yeah, to be perfectly honest I am not completely sure about this yet either. It's definitely a trade-off. More iterations might provide a higher confidence level, but will also take longer and longer to run. And memory leak testing is quite expensive in terms of time consumption...

I think we still need more experience in using leakage for memory leak testing. The lib is not even one month old, so for me it is hard to tell as well.

My experience so far: The heap snapshotting/diffing works quite precisely, except for the first snapshot (still got to debug why) and it triggers a full GC before snapshotting, providing almost noise-free results.

I am quite confident that even few iterations work well for simple cases. I would assume it works for more complex tests just as fine, since a heap snapshot is a heap snapshot, no matter how much heap operations have been done and the test fails if the heap size increased, regardless of what made it grow.

Please share your experiences any time! :)

andywer commented 7 years ago

Update: The new version that is about to be released comes with a slightly changed API.

In future you won't have to pass the iterations count anymore. You can pass an options object and set a custom value, though, if you need to. But I think the default values (30 x 6) work pretty good and will make this issue way less important.

So I will close the issue as soon as the new version is released. If you feel that this issue is still important, share your thoughts any time 😉

andywer commented 7 years ago

Leakage 0.3 is out and the new API will make this topic less interesting. It is also mentioned in the readme's FAQ.