rfc: Wouldn't UDP be able to benefit from a sendv() function?

[ronkorving]

Right now if you want to send multiple buffers, you have to concatenate them before calling send(). It seems however that libuv is perfectly fine with receiving a bunch of buffers. I can imagine that in particular use cases, this could speed up message delivery tremendously. Would love to hear thoughts on this.

[Fishrock123]

cc @mcollina

[mcollina]

I absolutely agree! This is supported by libuv, so I think we should add it here as well. Also, the performance of dgram is quite bad, so this would help a lot.

However, I think we should have a better send function, rather than introducing a sendv function.

What I would like to achieve is a send function with the following signatures:

• socket.send(buf, offset, length, port, address[, callback]) - backward compatibility, the offset and length parameters are legacy code from an old node version that did not have buffers. I would add a deprecation for this signature.
• socket.send(buf, urlObject, [, callback]) - where urlObject is what is returned from URL.parse, or a string, which is then parsed by URL.parse. Also this has the same signature for the rsinfo object when receiving a message, so a response is easily constructed.
• socket.send(buf, port, host, [, callback]) - same as before, but with splitted port/host.
• socket.send([buf1, buf2, ... ], urlObject [, callback]) - as @ronkorving said.
• socket.send([buf1, buf2, ... ], port, host, [, callback]) - same as before, but with splitted port/host.

I can work on this. Anybody has any opinions on this job?

Thanks for ccing me @Fishrock123 :).

[mscdex]

FWIW I'm -1 on deprecating the offset, length parameters because having those parameters makes things more efficient when you want to send a subset of data from a larger Buffer.

[mcollina]

@mscdex I never got the use case for that. 100% of my usage with udp is allocating a buffer and copying stuff in it, and then send. You usually need to add some header, and do some data manipulation on the input. Having the multiple buffer form will be super handy in those cases.

IMHO, offset, length is not consistent with most of the node api: stream.write() does not have an offset and length. Anyway, I'm suggesting to deprecate it at some point, but we would need to keep it around for long for backward compatibility.

@mscdex what is your opinion on the rest of the changes?

[mscdex]

Yes, it's unfortunate more of the node API doesn't support avoiding slicing by allowing offset, length arguments. I personally have encountered cases while creating protocol modules where I only want to send a subset of a Buffer that I already have. While the overhead of slicing is smaller than having to copy the data too, it's still overhead that could be avoided for such use cases.

The rest of the suggested changes look fine to me.

[ronkorving]

@mcollina Thanks for being onboard with this! I love your further proposal. Let me know if there is anything I can do to help.

[mcollina]

@nodejs/collaborators any opinions on this? It would be good to have some consensus on the direction we should take. We are missing a dgram/udp wg anyway, sorry for spamming everybody.

@ronkorving all the help you can give :D. Let's see what the others thinks about this, and then we can chat how we collaborate to get this done.

Currently we are missing a lot of benchmarks on dgram, and they are part of the net folder: https://github.com/nodejs/node/blob/master/benchmark/net/dgram.js. It is possibly worth adding a couple more (at least one involving a domain lookup, as that is done for each message), so that we do not create performance regressions. Also moving them out of net would make sense for me.

Also cc @trevnorris as for our nodeconf.eu discussion.

[indutny]

Sounds terrific! I'm all in for this.

[ronkorving]

More benchmarks would definitely be great. We need to be able to measure that what we do pays off. To compare though, when I implemented a similar solution in fs.WriteStream the benefits (depending on buffer size) would go up to 100-fold. I can imagine it being even higher in the case of UDP, since it doesn't depend on disk-latency.

Moving them out of /net makes sense indeed.

[mcollina]

Yes, it's unfortunate more of the node API doesn't support avoiding slicing by allowing offset, length arguments. I personally have encountered cases while creating protocol modules where I only want to send a subset of a Buffer that I already have. While the overhead of slicing is smaller than having to copy the data too, it's still overhead that could be avoided for such use cases.

I agree on the general topic. Did you experience this with dgram? The reason why I am asking is because of udp nature of messages, as you would need headers for every message you send, and in the current API you are forced to allocate a new packet and do a buffer.copy(), with all the downsides.

[mcollina]

@indutny thanks for joining the dgram discussion. Are you ok with adding some more API signatures to send() as well? I have a question for you: my goal is to to use the signature socket.send(buf, urlObject, [, callback]) to wrap the DTLS context in the urlObject parameter (and rsinfo when receiving a message`, so the API looks the same. (See #2398 (comment)). I would like the new API to be forward-looking, and allow users to support DTLS without a full API change.

[indutny]

@mcollina I need more time to make a decision on this, but as something preliminary I wonder if it may be a good idea to introduce cork/uncork as we have in regular streams right now?

socket.cork();
socket.send(...);
socket.send(...);
socket.uncork();  // will actually do a single `sendv` call

[saghul]

I'm generally +1 on this. Quick question @mcollina, what's that urlObject about? We need the IP address and port, an URL object looks incorrect to me.