Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1331)

Issue 9462044: code review 9462044: io: Prioritize WriterTos over ReaderFroms in Copy. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by DMorsing
Modified:
5 years, 1 month ago
Reviewers:
CC:
golang-dev, dsymonds, remyoudompheng, bradfitz, minux1
Visibility:
Public.

Description

io: Prioritize WriterTos over ReaderFroms in Copy. This only affects calls where both ReaderFrom and WriterTo are implemented. WriterTo can issue one large write, while ReaderFrom must Read until EOF, potentially reallocating when out of memory. With one large Write, the Writer only needs to allocate once. This also helps in ioutil.Discard since we can avoid copying memory when the Reader implements WriterTo.

Patch Set 1 #

Patch Set 2 : diff -r 4e860d4a312b https://code.google.com/p/go/ #

Patch Set 3 : diff -r 4e860d4a312b https://code.google.com/p/go/ #

Total comments: 2

Patch Set 4 : diff -r 4e860d4a312b https://code.google.com/p/go/ #

Patch Set 5 : diff -r 0fb55e40bd0c https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -8 lines) Patch
M src/pkg/io/io.go View 1 2 3 1 chunk +8 lines, -8 lines 0 comments Download
M src/pkg/io/io_test.go View 1 2 3 1 chunk +26 lines, -0 lines 0 comments Download

Messages

Total messages: 8
DMorsing
Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org, minux.ma@gmail.com), I'd like you to review this change to https://code.google.com/p/go/
5 years, 1 month ago (2013-05-20 20:24:21 UTC) #1
dsymonds
Test please.
5 years, 1 month ago (2013-05-20 20:25:36 UTC) #2
remyoudompheng
https://codereview.appspot.com/9462044/diff/4001/src/pkg/io/io.go File src/pkg/io/io.go (right): https://codereview.appspot.com/9462044/diff/4001/src/pkg/io/io.go#newcode334 src/pkg/io/io.go:334: // Otherwise, if src implements the WriterTo interface, the ...
5 years, 1 month ago (2013-05-20 20:37:36 UTC) #3
DMorsing
PTAL. The documentation change is making me wonder if I can even do this without ...
5 years, 1 month ago (2013-05-20 21:01:43 UTC) #4
remyoudompheng
On 2013/05/20 21:01:43, DMorsing wrote: > PTAL. > > The documentation change is making me ...
5 years, 1 month ago (2013-05-20 21:10:52 UTC) #5
DMorsing
On 2013/05/20 21:10:52, remyoudompheng wrote: > On 2013/05/20 21:01:43, DMorsing wrote: > > PTAL. > ...
5 years, 1 month ago (2013-05-21 06:55:05 UTC) #6
bradfitz
LGTM I've been trying to think of compatibility problems this could introduce, but I haven't ...
5 years, 1 month ago (2013-05-21 15:20:47 UTC) #7
DMorsing
5 years, 1 month ago (2013-05-23 16:29:27 UTC) #8
*** Submitted as https://code.google.com/p/go/source/detail?r=bb92bbe623fa ***

io: Prioritize WriterTos over ReaderFroms in Copy.

This only affects calls where both ReaderFrom and WriterTo are implemented.
WriterTo can issue one large write, while ReaderFrom must Read until EOF,
potentially reallocating when out of memory. With one large Write, the Writer
only needs to allocate once.

This also helps in ioutil.Discard since we can avoid copying memory when the
Reader implements WriterTo.

R=golang-dev, dsymonds, remyoudompheng, bradfitz
CC=golang-dev, minux.ma
https://codereview.appspot.com/9462044
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b