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
Labels
Documentation NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Performance
Milestone

Comments

@dsnet
Copy link
Member

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
	}
	...
}
@dsnet dsnet added Performance NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Dec 4, 2020
@cherrymui
Copy link
Member

(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 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

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
Labels
Documentation NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Performance
Projects
None yet
Development

No branches or pull requests

4 participants