|
|
|
Created:
14 years, 5 months ago by msolo Modified:
14 years, 5 months ago Reviewers:
CC:
bradfitz, rsc, golang-dev Visibility:
Public. |
io/ioutil: improve performance of ioutil.Discard
Fixes issue 2084.
Patch Set 1 #Patch Set 2 : diff -r f2765b9aec9b https://go.googlecode.com/hg #
Total comments: 2
Patch Set 3 : diff -r f2765b9aec9b https://go.googlecode.com/hg #
Total comments: 2
Patch Set 4 : diff -r f2765b9aec9b https://go.googlecode.com/hg #
Total comments: 2
Patch Set 5 : diff -r f2765b9aec9b https://go.googlecode.com/hg #
Total comments: 2
Patch Set 6 : diff -r f2765b9aec9b https://go.googlecode.com/hg #Total messages: 13
|
msolo
Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg
|
14 years, 5 months ago (2011年07月20日 20:13:39 UTC) #1 |
Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg
> + junk := blackHole[0:cap(blackHole)] > + readSize, err = r.Read(junk) you can just say readSize, err = r.Read(blackHole) there's no need for junk.
http://codereview.appspot.com/4817041/diff/2001/src/pkg/io/ioutil/ioutil.go File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/4817041/diff/2001/src/pkg/io/ioutil/ioutil.go#n... src/pkg/io/ioutil/ioutil.go:122: return n, nil why is there a for loop if you always exit on the first pass? this line is repeated on line 125. just remove this and the for? but I suspect you mean to actually loop and consume everything from r until an error, right?
Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
http://codereview.appspot.com/4817041/diff/7001/src/pkg/io/ioutil/ioutil.go File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/4817041/diff/7001/src/pkg/io/ioutil/ioutil.go#n... src/pkg/io/ioutil/ioutil.go:114: readSize := 0 you could ditch this line too and use := on line 116 http://codereview.appspot.com/4817041/diff/7001/src/pkg/io/ioutil/ioutil.go#n... src/pkg/io/ioutil/ioutil.go:121: return n, nil why is there a for loop if you always exit on the first pass? this line is repeated on line 125. just remove this and the for? but I suspect you mean to actually loop and consume everything from r until an error, right?
Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
http://codereview.appspot.com/4817041/diff/2001/src/pkg/io/ioutil/ioutil.go File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/4817041/diff/2001/src/pkg/io/ioutil/ioutil.go#n... src/pkg/io/ioutil/ioutil.go:122: return n, nil Good catch. I've fixed the loop. On 2011年07月20日 20:21:49, bradfitz wrote: > why is there a for loop if you always exit on the first pass? > > this line is repeated on line 125. just remove this and the for? but I suspect > you mean to actually loop and consume everything from r until an error, right?
http://codereview.appspot.com/4817041/diff/10001/src/pkg/io/ioutil/ioutil.go File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/4817041/diff/10001/src/pkg/io/ioutil/ioutil.go#... src/pkg/io/ioutil/ioutil.go:114: readSize := 0 either ditch this line, or... http://codereview.appspot.com/4817041/diff/10001/src/pkg/io/ioutil/ioutil.go#... src/pkg/io/ioutil/ioutil.go:122: return n, err .. or just make this line be "return" (and unident it, killing the else block) You seem to be going out of your way to avoid using := on line 116, but then you're not taking advantage of it.
Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
http://codereview.appspot.com/4817041/diff/14001/src/pkg/io/ioutil/ioutil.go File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/4817041/diff/14001/src/pkg/io/ioutil/ioutil.go#... src/pkg/io/ioutil/ioutil.go:120: return n, nil // err is EOF, so return nil explicitly err = nil (no comment necessary, because that statement is more explicit) http://codereview.appspot.com/4817041/diff/14001/src/pkg/io/ioutil/ioutil.go#... src/pkg/io/ioutil/ioutil.go:122: return n, err return (no arguments needed, because you're using the same n and err already)
Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
LGTM
*** Submitted as http://code.google.com/p/go/source/detail?r=13faa632ba3a *** io/ioutil: improve performance of ioutil.Discard Fixes issue 2084. R=bradfitz, rsc CC=golang-dev http://codereview.appspot.com/4817041 Committer: Brad Fitzpatrick <bradfitz@golang.org>