Skip to content
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

bytes: clarify semantics regarding Buffer.Bytes and Buffer.Write #42986

Open
dsnet opened this issue Dec 4, 2020 · 3 comments
Open

bytes: clarify semantics regarding Buffer.Bytes and Buffer.Write #42986

dsnet opened this issue Dec 4, 2020 · 3 comments

Comments

@dsnet
Copy link
Member

@dsnet dsnet commented Dec 4, 2020

Buffer.Bytes is currently documented as:

Bytes returns a slice of length b.Len() holding the unread portion of the buffer.
The slice is valid for use only until the next buffer modification (that is,
only until the next call to a method like Read, Write, Reset, or Truncate).
The slice aliases the buffer content at least until the next buffer modification,
so immediate changes to the slice will affect the result of future reads.

The documented use case for Bytes is that it's a view of the unread portion. If so, then I would expect len(b) == cap(b) for the returned buffer, but its not. Instead, the buffered returned is both the unread portion in b[:len(b)] and the unwritten portion in b[len(b):].

Thus, you can do something like:

bb := bytes.NewBuffer(make([]byte, 0, 1024))
b := bb.Bytes()
b = strconv.AppendFloat(b[len(b):], math.Pi, 'e', -1, 64)
bb.Write(b)

Is this is an intended use-case for Bytes? From the implementation, I see no reason why this is invalid nor any significant hindrances it would impose on the internal implementation of Buffer. This technique allows users of Buffer to avoid allocating a temporary local buffer and an unnecessary copy if they already wrote the data directly into the internal buffer.

EDIT: The suggestion below is not necessary as pointed out by @cherrymui.
If this is an appropriate use of the Bytes method, then I suggest that Write have a special case where it trivially reslices the internal buffer if the input slice aliases the next buffer segment:

func (b *Buffer) Write(p []byte) (n int, err error) {
	// If the input is the next segment of the internal buffer (as obtained by Bytes),
	// then trivially reslice the internal buffer to include the next segment.
	if cap(b.buf) > len(b.buf) && len(p) > 0 && &b.buf[:cap(b.buf)][len(b.buf)] == &p[0] {
		b.buf = b.buf[:len(b.buf)+len(p)]
		return len(p), nil
	}
	...
}
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Dec 4, 2020

(I'm not sure I really understand the use case and not sure about about what semantics Buffer.Bytes should be. Just a comment about the performance part.)

If this is an appropriate use of the Bytes method, then I suggest that Write have a special case where it trivially reslices the internal buffer if the input slice aliases the next buffer segment

Is this necessary? If the input slice aliases the next buffer segment, the current code should already do that, as self-copy should return immediately.

@dsnet
Copy link
Member Author

@dsnet dsnet commented Dec 4, 2020

Is this necessary? If the input slice aliases the next buffer segment, the current code should already do that, as self-copy should return immediately.

Great, I wasn't aware that copy had this optimization already.

Now this issue is purely a documentation one.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 7, 2021

Change https://golang.org/cl/308229 mentions this issue: bytes: document Buffer.Bytes semantics more clearly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants