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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 10 months ago by DMorsing
Modified:
10 years, 10 months 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/
10 years, 10 months ago (2013-05-20 20:24:21 UTC) #1
dsymonds
Test please.
10 years, 10 months 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 ...
10 years, 10 months 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 ...
10 years, 10 months 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 ...
10 years, 10 months 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. > ...
10 years, 10 months 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 ...
10 years, 10 months ago (2013-05-21 15:20:47 UTC) #7
DMorsing
10 years, 10 months 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