New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: add Buffers.ReadFrom and Buffers.Write? #17607

Open
rsc opened this Issue Oct 26, 2016 · 10 comments

Comments

Projects
None yet
8 participants
@rsc
Contributor

rsc commented Oct 26, 2016

Go 1.8 is adding net.Buffers, with Read and WriteTo methods. These both move data from the buffers to a target, either a []byte (for Read) or a Writer (for WriteTo). The implementation of WriteTo on one of the package net-implemented net.Conns uses writev. So far so good.

What about readv? What is that going to look like? I ask because it might influence some of the finer details of Buffers. For example, if we can't support readv with the same data structure, we may want to call it WriteBuffers and have a separate ReadBuffers. I do think we can support readv, but we should make sure.

For example you could imagine that readv works by setting up a Buffers with space in the []bytes between len and cap, like:

b := Buffers{make([]byte, 0, 16), make([]byte, 0, 1024)}

to read the first 16 bytes into the first slice and the next 1k into the second, for both b.Write(data) and b.ReadFrom(conn). In order to allow repeating the reads until you get all the data you want, you would need to define that Write and ReadFrom (unlike Read and WriteTo) do not remove full slices from Buffers: they just top off what's available. Probably only top off the final non-empty buffer. That is, if you have

b := Buffers{make([]byte, 4, 10), make([]byte, 15, 20), make([]byte, 0, 1024)}

and you read 5 bytes, where do they go? It seems wrong for them to go into the first slice, since that would stick them in the middle of the 4+15 bytes already semantically in the buffer. So probably they go into the middle slice, which has 5 (20-15) spare bytes of capacity. This implies that Write/ReadFrom needs to scan backward from the end to find the first non-empty buffer.

It would be nice if Read/WriteTo, as they pull data out of the slices, didn't throw away the slices entirely, so that you could imagine using a Buffers not unlike a bytes.Buffer, where once allocated to a particular size you could repeatedly Write into it and then Read out from it (or ReadFrom into it and WriteTo out of it). Unfortunately even if the writing operations did avoid cutting slices out of the buffer, they need some way to track what is left to be written from a particular slice, and the way to do that is to advance the base pointer, reducing len and cap. At the end of a complete write the lens of the buffers are necessarily 0, and there's no way to back them back up. Concretely, if I have:

b := Buffers{make([]byte, 100)}

and I write 99 bytes, I'm left now with Buffers{slice(len=1,cap=1)}. If I write 100 bytes, I'm left with Buffers{} (no slices). The desire to reuse a Buffer would suggest that maybe Read/WriteTo shouldn't pull slices out, but it's not terribly useful to leave them in when they'd have len (and likely cap) 0 so they are basically useless anyway.

This implies that reuse of a Buffers for new reading into the buffers after writing from the buffers is probably just hard. You'd need to do something like:

allBuffers := Buffers{make([]byte, 0, 100), make([]byte, 0, 100)}
b := make(Buffers, len(allBuffers))
for {
    copy(b, allBuffers) // restore full capacity
    b.ReadFrom(conn1)
    b.WriteTo(conn2)
}

That seems OK to me. I just want to make sure we've thought about all this.

@bradfitz, what do you think? Does this seem like a reasonable basic plan for readv? Should we do it for Go 1.8 so that there's never a net.Buffers that only works for writev?

@rsc rsc added this to the Go1.8 milestone Oct 26, 2016

@rsc rsc added the NeedsDecision label Oct 26, 2016

@nussjustin

This comment has been minimized.

Contributor

nussjustin commented Oct 26, 2016

FWIW I played with implementing readv (ReadFrom/Write) once (see cl/30102) and got some nice numbers (which I don't have anymore) in my tests. My implementation kept the current type and all calls to Write/ReadFrom just overwrote the old data. I didn't have a real use for it back than (and still currently don't have), but from the raw numbers I got it seemed like it could be easily used to speed up some tools (probably stuff like parsers, etc.)

You'd need to do something like:

As I understand your code ReadFrom would need to reslice net.Buffers to make WriteTo work properly. But what about Write? Would it append to the net.Buffers (and potentially adding a new slice) or only write as much as fits and return an error otherwise? If it appends a new slice users would need to remember to reuse it. Although Read/Write on net.Buffers will probably rarely be used, I'd like to have this clearly defined and though about.

@bradfitz

This comment has been minimized.

Member

bradfitz commented Oct 26, 2016

Yeah, that copy is what I figured people caring about allocations could do.

I don't care whether readv support goes into Go 1.8 or later, though. @nuss-justin and @ianlancetaylor also seemed lukewarm on it. @ianlancetaylor said on that CL:

In a language like Go readv seems much less interesting than writev. It seems easy enough to read into a slice and then use slice expressions to pass around different versions to different places

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Oct 26, 2016

As far as I can see, the only meaningful reason to use readv in Go is to assemble a list of slices into existing arrays, and read into them. So examples like Buffers{make([]byte, 0, 16)} seem to me to be somewhat uninteresting. What you really do is Buffers{hdr[:16], data[:1024]}. In that mode, what the Buffers type looks like after Buffers.ReadFrom(conn) does not seem very interesting. All you really need to know is how much data you got.

I guess I don't see why anybody would want to use the same Buffers value for reading and writing. In the cases where a scatter-read is valuable, I'm not going to want to do a gather-write from the same set of buffers. If I wanted to send the data through I would just use a straight []byte and use byte slices to analyze it as needed.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 3, 2016

@ianlancetaylor, I don't know that anyone would necessarily want to reuse the Buffers type, but it seemed worth thinking through what it might look like.

I do want to make sure that we know how Buffers will support readv, or else that we know we will never want to support readv. If readv will need something different, the "Buffers" may not be the right name. I agree that Buffers{hdr[:16], data[:1024]} is the interesting case. However, it matters what the Buffers looks like after ReadFrom(conn) because you might expect to get everything in one read but actually need two reads. The second b.ReadFrom(conn) needs to correctly pick up where the previous one left off.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 3, 2016

It does sound like we agree that the current Buffers definition has a reasonable way to support readv, though, so I'll punt the actual addition of readv to Go 1.9.

@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 3, 2016

@rsc rsc added NeedsFix and removed NeedsDecision labels Nov 3, 2016

@noblehng

This comment has been minimized.

noblehng commented Feb 21, 2017

Making the Buffer a ring buffer seems better for reusing it even only for writev.

And what about sendmmsg/recvmmsg? They are more general than writev/readv but not as portable. Unlike writev, sendmmsg can write to unconnected sockets and multiple addresses, these could be useful for udp servers like QUIC. For now, you can use Buffer to write to a listening UDPConn, but will only get a error after doing the actual writev syscall, that matches the Write() behavior of UDPConn, so I guess it is not a problem.

@DarienRaymond

This comment has been minimized.

Contributor

DarienRaymond commented Apr 21, 2017

+1 for supporting readv in net.Conn

I am working on a network proxy project. A proxy workflow (per connection) now looks like below:

  1. Get a buffer ([]byte) from a sync.Pool
  2. Fill in the buffer from an inbound connection.
  3. Process the content (header, encoding, etc)
  4. Send out the buffer through an outbound connection.
  5. Release the buffer back to the pool.
  6. Repeat until connection ends.

The problem is that it is difficult to determine the size of the buffer. Using a 32K or larger buffer can increase throughput for a single connection with large payload. But it increases (unnecessary) memory usage when there are many connections with little payload.

With net.Buffers.ReadFrom, I can do the following:

  1. Get 10 of [2k]byte from the pool
  2. Fill them in. Keep only the buffers that has content, say [5][2k]byte, and return the rest to the pool.
  3. Process in the same way as usual.

In this way, the memory usage can automatic adapt for different network scenarios.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 9, 2017

@bradfitz bradfitz modified the milestones: Go1.10, Unplanned Nov 28, 2017

@anacrolix

This comment has been minimized.

Contributor

anacrolix commented Jan 8, 2018

Can someone comment on the recvmmsg usage?

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Sep 7, 2018

I don't see how the existing net.Buffers type can be used reasonable with the sendmmsg and recvmmsg system calls. Those calls include peer information that can not be represented in net.Buffers at all. I think we need a different mechanism.

@VictoriaRaymond

This comment has been minimized.

VictoriaRaymond commented Sep 7, 2018

A quick follow-up on this. We have implemented our own version of readv() in V2Ray, based on syscall.RawConn interface. As of Go 1.11, the reader works on all major operation systems.

The library is coupled with the rest of the project for the moment. If others are interested, we can make it standalone package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment