Kad Records could use value: Bytes

[joshuef]

So I've https://github.com/joshuef/rust-libp2p/tree/MakeKadRecordValuebytes trying to use Bytes instead of Vec in the Record struct.

My thinking being that we (https://github.com/maidsafe/safe_network) are using libp2p/kad/record_store probably beyond what was initially envisaged (our record size is up to 1mb). And this seems like we're hitting a lot of mem when a lot of queries come in.

Now one angle might be reducing that max size, which we'll be looking at. But I was also wondering about caching at the record_store layer (as requests for the same data come in relatively fast bursts... I assume due to the nature of kad?), but as each record is a Vec it'll need cloning and so be duplicating that same data over and over....

So if records hold Bytes we could send along our same Bytes (I'm not sure if that will be undone at some other layer? I believe quinn wants Bytes in the end though, so it could potentially have some good mem savings for repeat requests...

Is there anything off in my reasoning above? Is there perhaps a better way to approach this? Or are there other reasons this might not be desirable?

(As ever thanks to the whole team for all the work so far 🙇 !)

[thomaseizinger]

Those bytes will be duplicated once they are written to the stream inside the kademlia ConnectionHandler because we need to construct the kad protobuf message.

It might be possible to cache those messages but how would we invalidate this cache?

From there, you might have more allocations depending on the underlying transport. For quinn, we still need to send messages to the actual task that connects the logic streams to the IO source. For yamux, something similar applies.

Unfortunately, doing this zero-copy is very hard due to how AsyncWrite is designed. It takes a byte slice and we'd have to directly write to the socket from there. We also need to encrypt stuff (noise/tls).

Improvements are welcome but need to come with benchmarks :)

[joshuef]

Aha, yeh I thought it would be muuuch more complex than that!

(I was thinking to cache at the app layer for certain larger message types)

Okay good to get some background on it, but I don't think I'll push this further just yet. I'll take some time and read around the kad protobuf more and how transports are working in general.

Thanks for the pointers 👍

[thomaseizinger]

No worries at all. I'll close this as completed then because there is no immediate action :)