Huge increase in memory allocation

[stormshield-frb]

Summary

We have a server dedicated to stress testing our network and since the recent rebase of our fork with the libp2p master branch, this server gets OOM killed a lot.

In order to find out what was happening, our team has discovered that the configurability of the Kademlia bucket size (#5414) has introduced a huge increase in memory allocation, which imply an increase in memory fragmentation.

More precisely, it seems to be related to this code change : 417968e#diff-25d9554b592e6280d215bfc8b0bdb5a61eea3a6ca795849e6c7400414cb00c1cR288.

Previously, there was no heap allocation but now there is.

Expected behavior

Don't have heap allocation at every get_closest_peers request.

Actual behavior

An allocation happens at every get_closest_peers request.

Relevant log output

No response

Possible Solution

Maybe use a SmallVec with the default K_VALUE in order to have the same behaviour than before ?

Or maybe use const generic ? It would not allow users to define the K_VALUE at runtime but it would allow them to define it in there code base.

Version

No response

Would you like to work on fixing this bug ?

Maybe

[guillaumemichel]

The SmallVec solution sounds good. So only systems with a bucket size larger than K_VALUE will actually pay the heap allocation price?

[stormshield-frb]

Yeah that is the idea. At least on our part because we don't plan to change the K_VALUE so we just want to go back to how it was 😂 But I don't know if it would be an acceptable solution for people wishing to change the K_VALUE.

[guillaumemichel]

If users wish to reduce the bucket size, they wouldn't be penalized.

And for those willing to have a higher bucket size, are there alternatives to allow defining the bucket size at run time?

[stormshield-frb]

I don't think so. Personally, I only see the SmallVec solution and the one were we define a const generic argument on the Kad behaviour. But maybe there is and I did not think about it.

On our end we are going to patch our fork with the SmallVec solution for now to mitigate the effect quickly. But we can take a little more time to have some other opinions before choosing what to do for the permanent solution. What do you think ? Maybe other contributors will propose a better solution.

The question is what could be the maximum smart K_VALUE value that someone could define ? If it is 20 or not much higher, we put this value (20, 25?) in the SmallVec I think. If it is much higher, then the people wishing to do that would still be able to change the value in a patch at compile time.

[guillaumemichel]

This sounds good! I am not aware of any live kademlia system using a value larger than 20 as bucket size. If it becomes useful at some point to have larger bucket values for some users, we can reevaluate.