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

Jump to bottom

bytes, string: add Reset method to bytes.Reader and strings.Reader #15033

Closed
dsnet opened this issue Mar 30, 2016 · 7 comments
Closed

bytes, string: add Reset method to bytes.Reader and strings.Reader #15033

dsnet opened this issue Mar 30, 2016 · 7 comments
Labels
FrozenDueToAge
Milestone
Go1.7

Comments

@dsnet
Copy link
Member

dsnet commented Mar 30, 2016

Using go1.6

Currently, there is no easy allocation-free way to turn a []byte or string into an io.Reader. Adding a Reset method would allow this to easily done. It would also be consistent with the fact that many io.Readers in the standard library already have a Reset method of some sort: bytes.Buffer, bufio.Reader, gzip.Reader, flate.Reader, etc.
@bradfitz
Copy link
Contributor

A Reset on bytes.Reader and strings.Reader seems fine, and it would reduce allocations if you're repeatedly needing the Reader, but when does that occur? You'd still need to allocate the strings.Reader in the first place (40 bytes). Coincidentally, creating an io.SectionReader to get a Reader out of the strings.Reader (which is an io.ReaderAt) is also 40 bytes.

Anyway, seems fine, but I'm curious about your needs more.

@dsnet
Copy link
Member Author

dsnet commented Mar 31, 2016

I had two in mind, but I'm sure there are more.

  1. Embed it in a struct and Reset it as needed:
type OtherReader struct {
    // Other fields.    

    strRd strings.Reader
}

func (or *OtherReader) someFunc() {
    or.strRd.Reset(buf)
    // Do stuff with or.stdRd
}
  1. Use it in benchmarks to avoid allocation (that way any allocations seen occur because of the object we are actually benchmarking):
var byteRd bytes.Reader
for i := 0; i < b.N; i++ {
    byteRd.Reset(testBuf)
    or.Reset(&byteRd)
    io.Copy(ioutil.Discard, or)
}

@bradfitz
Copy link
Contributor

Oh, I thought you just wanted to seek back to the beginning:

func (r *Reader) Reset() {
    r.i = 0
    r.prevRune = -1

But in retrospect, we already have Seek for that.

Changing the source of the Reader is a bit more of a chance, but it's true that it's already consistent with...

go1.1.txt:pkg time, method (*Timer) Reset(Duration) bool
go1.2.txt:pkg bufio, method (*Reader) Reset(io.Reader)
go1.2.txt:pkg bufio, method (*Writer) Reset(io.Writer)
go1.2.txt:pkg compress/flate, method (*Writer) Reset(io.Writer)
go1.2.txt:pkg compress/gzip, method (*Writer) Reset(io.Writer)
go1.2.txt:pkg compress/zlib, method (*Writer) Reset(io.Writer)
go1.3.txt:pkg compress/gzip, method (*Reader) Reset(io.Reader) error
go1.4.txt:# CL 97140043 compress/flate: add Reset() to allow reusing large buffers to compress multiple buffers, James Robinson <jamesr@google.com>
go1.4.txt:pkg compress/flate, type Resetter interface { Reset }
go1.4.txt:pkg compress/flate, type Resetter interface, Reset(io.Reader, []uint8) error
go1.4.txt:pkg compress/zlib, type Resetter interface { Reset }
go1.4.txt:pkg compress/zlib, type Resetter interface, Reset(io.Reader, []uint8) error
go1.5.txt:pkg debug/dwarf, method (*LineReader) Reset()
go1.5.txt:pkg os/signal, func Reset(...os.Signal)
go1.txt:pkg bytes, method (*Buffer) Reset()
go1.txt:pkg crypto/rc4, method (*Cipher) Reset()
go1.txt:pkg go/scanner, method (*ErrorList) Reset()

So, sure. Send a change?

@gopherbot
Copy link

CL https://golang.org/cl/21386 mentions this issue.

@bradfitz bradfitz added this to the Go1.7 milestone Apr 7, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Apr 7, 2016

This wasn't auto-closed for some reason.

@bradfitz bradfitz closed this as completed Apr 7, 2016
@dsnet
Copy link
Member Author

dsnet commented Apr 7, 2016

I think it's because the CL wasn't submitted?

@bradfitz
Copy link
Contributor

bradfitz commented Apr 7, 2016

Whoops. Not sure what I was reading.

@bradfitz bradfitz reopened this Apr 7, 2016
@golang golang locked and limited conversation to collaborators Apr 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7
Development

No branches or pull requests
3 participants
@bradfitz @dsnet @gopherbot