Shouldn't socket recv() promise in its doc allows a nicer API

[keepsimple1]

socket2/src/socket.rs

Lines 409 to 414 in c93cdcc

	/// Normally casting a `&mut [u8]` to `&mut [MaybeUninit<u8>]` would be
	/// unsound, as that allows us to write uninitialised bytes to the buffer.
	/// However this implementation promises to not write uninitialised bytes to
	/// the `buf`fer and passes it directly to `recv(2)` system call. This
	/// promise ensures that this function can be called using a `buf`fer of
	/// type `&mut [u8]`.

A friendly question: because of the above promise, is it true that this API could change its signature to accept buf: &mut [u8] ? That will be so much helpful! (The current API is useless for many code bases where unsafe is not allowed / or strongly discouraged).

(I noticed open issues like #366 , but it seems being there more than 1 year, when will it happen?) I'm a user of this crate and I think this current signature is really unnecessary hurdle for its users.

[Thomasdezeeuw]

You can use the std::io::Read implementation for this. It makes a read(2) system call instead of send(2), but those behave the same way with the arguments we pass.

As for the recv method itself, it will continue to use to uninitialised bytes to not force the caller to initialise them only to overwrite them again. Using the readbuf API would be nice, but that's not stable, so not usable by this crate.

[keepsimple1]

thanks for your reply. I should have, but forgot to clarify that, what I needed is .recv_from() which is in the same boat as .recv() (and doc forwarded to .recv()) .

Currently I'm using read(), but won't be able to get the source IP.

[Thomasdezeeuw]

For recv_from we don't have a version that uses &[u8]. We've had attempts to to add them, e.g. #246, but it basically came down to coping all the recv methods only to call the version with uninitialised bytes, so it wasn't worth the maintenance burden.

[keepsimple1]

I guess it was probably a tough decision for you the maintainers, but I was curious how much performance gain of the version with uninitialized buffer. Among all socket2 users, how many percent of users prefer to use this version of uninitialized buffer?

It's probably impossible to change it back, but I just felt it's not a worthy trade-off, i.e. disabling / crippling common use cases for (IMO) edge cases.

[Thomasdezeeuw]

I guess it was probably a tough decision for you the maintainers, but I was curious how much performance gain of the version with uninitialized buffer. Among all socket2 users, how many percent of users prefer to use this version of uninitialized buffer?

I don't really have performance test ready, but here an example of using uninitialised memory for HTTP headers: https://github.com/5225225/hyper/commit/325b7e519a2abb00c3ac95486ebfad14f0119ab2 that saves ~5%.

It's probably impossible to change it back, but I just felt it's not a worthy trade-off, i.e. disabling / crippling common use cases for (IMO) edge cases.

It's not really the common case though. Most users of this library use advance features, where not initialising memory is considered the common case.

[keepsimple1]

I don't really have performance test ready, but here an example of using uninitialised memory for HTTP headers: 5225225/hyper@325b7e5 that saves ~5%.

5% seems to me really not worth changing a safe Rust code to unsafe Rust code, especially if in the grand scheme of a bigger system.

[Thomasdezeeuw]

5% is a lot actually. That's massive improvement. Especially for a 1 line change that is documented as supported by socket2.

I think the best way forward is the read buf, which is still unstable. So closing in favour of #366.