[go] io: NopCloser forward WriterTo implementations if the reader supports it
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http's NopCloser unwrapping logic.
The io_test.go don't bother to test actually reading or WritingTo of the NopCloser because the logic is simple. NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 93f6af4648fb17a669d606bf96a32bb3d645cac9
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 68 insertions(+), 5 deletions(-)
diff --git a/src/io/io.go b/src/io/io.go
index 1ea01d5..56e8325 100644
--- a/src/io/io.go
+++ b/src/io/io.go
@@ -621,8 +621,16 @@
// NopCloser returns a ReadCloser with a no-op Close method wrapping
// the provided Reader r.
+// If r implements WriterTo, the returned ReadCloser will also and forward calls.
func NopCloser(r Reader) ReadCloser {
- return nopCloser{r}
+ c := nopCloser{r}
+ if to, ok := r.(WriterTo); ok {
+ return struct {
+ nopCloser
+ WriterTo
+ }{c, to}
+ }
+ return c
}
type nopCloser struct {
diff --git a/src/io/io_test.go b/src/io/io_test.go
index 3088460..067bd12 100644
--- a/src/io/io_test.go
+++ b/src/io/io_test.go
@@ -471,3 +471,24 @@
t.Errorf("Copy error: got %v, want %v", err, want)
}
}
+
+func TestNopCloserWriterToForwarding(t *testing.T) {
+ for _, tc := range [...]struct {
+ Name string
+ r Reader
+ }{
+ {"not a WriterTo", Reader(nil)},
+ {"a WriterTo", struct {
+ Reader
+ WriterTo
+ }{}},
+ } {
+ nc := NopCloser(tc.r)
+
+ _, expected := tc.r.(WriterTo)
+ _, got := nc.(WriterTo)
+ if expected != got {
+ t.Errorf("NopCloser incorrectly forwards WriterTo for %s, exepected %t; got %t", tc.Name, expected, got)
+ }
+ }
+}
diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index d9edf8c..86d0327 100644
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -422,8 +422,8 @@
//
// This function is only intended for use in writeBody.
func (t *transferWriter) unwrapBody() io.Reader {
- if reflect.TypeOf(t.Body) == nopCloserType {
- return reflect.ValueOf(t.Body).Field(0).Interface().(io.Reader)
+ if r, ok := unwrapNopCloser(t.Body); ok {
+ return r
}
if r, ok := t.Body.(*readTrackingBody); ok {
r.didRead = true
@@ -1081,6 +1081,23 @@
}
var nopCloserType = reflect.TypeOf(io.NopCloser(nil))
+var nopCloserWriterToAbleType = reflect.TypeOf(io.NopCloser(struct {
+ io.Reader
+ io.WriterTo
+}{}))
+
+// unwrapNopCloser return the underlying reader and true if r is a NopCloser
+// else it return false
+func unwrapNopCloser(r io.Reader) (underlyingReader io.Reader, isNopCloser bool) {
+ switch reflect.TypeOf(r) {
+ case nopCloserType:
+ return reflect.ValueOf(r).Field(0).Interface().(io.Reader), true
+ case nopCloserWriterToAbleType:
+ return reflect.ValueOf(r).Field(0).Field(0).Interface().(io.Reader), true
+ default:
+ return nil, false
+ }
+}
// isKnownInMemoryReader reports whether r is a type known to not
// block on Read. Its caller uses this as an optional optimization to
@@ -1090,8 +1107,8 @@
case *bytes.Reader, *bytes.Buffer, *strings.Reader:
return true
}
- if reflect.TypeOf(r) == nopCloserType {
- return isKnownInMemoryReader(reflect.ValueOf(r).Field(0).Interface().(io.Reader))
+ if r, ok := unwrapNopCloser(r); ok {
+ return isKnownInMemoryReader(r)
}
if r, ok := r.(*readTrackingBody); ok {
return isKnownInMemoryReader(r.ReadCloser)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer.
2 comments:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {I think it's probably worth working a little harder to avoid what I think is an unnecessary allocation here. I think we can use
type nopCloserWriterTo struct {
Reader
}
func (nopCloserWriterTo) Close() error { return nil }
func (c nopCloserWriterTo) WriteTo(w Writer) (n int64, err error) {
return c.Reader.(WriterTo).WriteTo(w)
}
func NopCloser(r Reader) ReadCloser {
if _, ok := r.(WriterTo); ok {
return nopCloserWriterTo{r}
}
return nopCloser{r}
}
File src/io/io_test.go:
Patch Set #1, Line 491:
exepected %t; got %t"We usually write "got %t want %t", as above.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {
I think it's probably worth working a little harder to avoid what I think is an unnecessary allocati […]
Which allocations ?
func BenchmarkNotWriterTo(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
runtime.KeepAlive(io.NopCloser(struct{io.Reader}{}))
}
}func BenchmarkWriterTo(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
runtime.KeepAlive(io.NopCloser(struct{io.Reader; io.WriterTo}{}))
}
}goos: linux
goarch: amd64
cpu: AMD Ryzen 5 3600 6-Core Processor
BenchmarkNotWriterTo-12 191579270 6.385 ns/op 0 B/op 0 allocs/op
BenchmarkWriterTo-12 220612800 5.531 ns/op 0 B/op 0 allocs/op
PASS
ok command-line-arguments 3.634s
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/io/io_test.go:
Patch Set #1, Line 491:
exepected %t; got %t"We usually write "got %t want %t", as above.
Done
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
Gerrit Bot uploaded patch set #2 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http's NopCloser unwrapping logic.
The io_test.go don't bother to test actually reading or WritingTo of the NopCloser because the logic is simple. NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: d38042cc8da27756c8b2a20b7544b2ca1c030487
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 68 insertions(+), 5 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Jorropo.
1 comment:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {
Which allocations ? […]
That's not a realistic benchmark, though, because nobody actually uses the type "struct{io.Reader; io.WriterTo}". The benchmark to test is a comparison of calling NopCloser with a type that only implements io.Reader and calling NopCloser with a type that implements both io.Reader and io.WriterTo.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {
That's not a realistic benchmark, though, because nobody actually uses the type "struct{io. […]
With incomplete knowlege of golang's virtual types system, I don't see why that would have an impact, AFAIK a more complex type would only change which types pointers are shuffled arround.
Here is a test using a *bytes.Reader instead that have 0 allocations:
goos: linux
goarch: amd64
cpu: AMD Ryzen 5 3600 6-Core Processor
BenchmarkNotWriterTo-12 181507446 6.651 ns/op 0 B/op 0 allocs/op
BenchmarkWriterTo-12 175321104 7.157 ns/op 0 B/op 0 allocs/op
type onlyReader struct {
io.Reader
}var data = []byte{1, 2, 3}
func BenchmarkNotWriterTo(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
runtime.KeepAlive(io.NopCloser(onlyReader{bytes.NewReader(data)}))
}
}
func BenchmarkWriterTo(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
r := bytes.NewReader(data)
runtime.KeepAlive(onlyReader{r}) // Do to match NotWriterTo in case the onlyReader{r} cast allocates (this is at 0 anyway so doesn't matter)
runtime.KeepAlive(io.NopCloser(r))
}
}
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {
With incomplete knowlege of golang's virtual types system, I don't see why that would have an impact […]
NVM it turns out that even tho I runtime.KeepAlive it this code got eliminated. MB
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {
NVM it turns out that even tho I runtime.KeepAlive it this code got eliminated. […]
So with a fixed benchmark, I have data that makes sense, it seems like the compiler coalles the two types allocations into one:
BenchmarkNotWriterTo-12 24693268 49.69 ns/op 16 B/op 1 allocs/op
BenchmarkWriterTo-12 19768785 61.31 ns/op 32 B/op 1 allocs/op
type onlyReader struct {
io.Reader
}var data = []byte{1, 2, 3}
var reader io.Reader = onlyReader{buff}
var buff io.Reader = bytes.NewReader(data)//go:noinline
func nopCloser(r io.Reader) io.Reader {
return io.NopCloser(r)
}
func BenchmarkNotWriterTo(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
nopCloser(reader)
}
}
func BenchmarkWriterTo(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
nopCloser(buff)
}
}I doubt there would be much impact saving 16 bytes on an allocation.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/io/io.go:
Patch Set #2, Line 631:
}{c, to}For the sake of debugging, I think it would be better for this to return a named nopWriteToCloser type:
if to, ok := r.(WriterTo); ok {
return nopCloserWriterTo{to}
}
return nopCloser{r}(With the obvious implementation of nopCloserWriterTo below.)
This way, if someone prints the type of the returned io.ReadCloser they see a descriptive type name rather than an anonymous struct type.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
2 comments:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {So with a fixed benchmark, I have data that makes sense, it seems like the compiler coalles the two […]
Done
However:
I failed to find any case where my original solution (embeding io.WriterTo) would do multiple allocations. All I can find is it allocates 2 uintptr more bytes in the single allocation.As far as I know the question here is:
Does saving 2 uintptr on all NopCloser worth one more call and WriterTo cast in nopCloserWriterTo.WriteTo function ?On my machine that overhead is ~5ns.
File src/io/io.go:
Patch Set #2, Line 631:
}{c, to}
For the sake of debugging, I think it would be better for this to return a named nopWriteToCloser ty […]
Done
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
Gerrit Bot uploaded patch set #3 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http's NopCloser unwrapping logic.
The io_test.go don't bother to test actually reading or WritingTo of the NopCloser because the logic is simple. NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 492c6c2ae572026921ef8de58408fc126ebcb58a
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 71 insertions(+), 4 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Jorropo.
Patch set 3:Run-TryBot +1
2 comments:
File src/io/io.go:
Patch Set #1, Line 627:
if to, ok := r.(WriterTo); ok {
Done […]
For the standard library I think it's worth doing the smaller allocation. Thanks.
File src/io/io.go:
Patch Set #3, Line 624:
// If r implements WriterTo, the returned ReadCloser will also and forward calls.This sentence is a bit confusing.
If r implements WriterTo, the returned ReadCloser will implement WriterTo by forwarding calls to r.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Jorropo.
Gerrit Bot uploaded patch set #4 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http's NopCloser unwrapping logic.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 492c6c2ae572026921ef8de58408fc126ebcb58a
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 72 insertions(+), 4 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
2 comments:
Patchset:
1 of 30 TryBots failed. […]
Rebased to master.
File src/io/io.go:
Patch Set #3, Line 624:
// If r implements WriterTo, the returned ReadCloser will also and forward calls.
This sentence is a bit confusing. […]
Done
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
Gerrit Bot uploaded patch set #5 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http's NopCloser unwrapping logic.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 50fe8e9c9c998e75c7c304b617e2f7f1590e7b35
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 73 insertions(+), 4 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer.
Patch set 5:Run-TryBot +1
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer.
1 comment:
Patchset:
1 of 30 TryBots failed. […]
Rebased to master, again. It looks like a flaky CI, I think it just need the right rerun.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer.
Gerrit Bot uploaded patch set #6 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http's NopCloser unwrapping logic.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: cfd9083f298d17a68745f43638983781c1efb790
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 73 insertions(+), 4 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Jorropo.
1 comment:
Patchset:
Rebased to master, again. It looks like a flaky CI, I think it just need the right rerun.
It doesn't look flaky to me. It looks directly related to this change. bytes.Reader.WriteTo does not permit concurrent calls. It may be that something has to change in the test, but it doesn't look flaky. You should be able to recreate the problem by running
go test -race -test.run=TestServerEmptyBodyRace net/http
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Jorropo.
Gerrit Bot uploaded patch set #7 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 9ccde47aed0a1c729d527bb3b4ba7dad0ea488a0
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/h2_bundle.go
M src/net/http/transfer.go
4 files changed, 81 insertions(+), 9 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
2 comments:
Patchset:
Rebased to master.
Done
Patchset:
It doesn't look flaky to me. It looks directly related to this change. bytes.Reader. […]
You are right. That is Fixed
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
Gerrit Bot uploaded patch set #8 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 3420b891a4619b712d08a51f100ef0bfca2eabe3
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/h2_bundle.go
M src/net/http/transfer.go
4 files changed, 81 insertions(+), 9 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer.
Patch set 8:Run-TryBot +1
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer.
1 comment:
File src/net/http/h2_bundle.go:
Patch Set #8, Line 33:
"io/ioutil"This is a generated file. Changes to this file must be made in the golang.org/x/net repo and then copied in here. Thanks, and sorry for the busywork.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/net/http/h2_bundle.go:
Patch Set #8, Line 33:
"io/ioutil"
This is a generated file. Changes to this file must be made in the golang. […]
Patch sent: https://go-review.googlesource.com/c/net/+/401014
Since there have been a few more (minor) changes since, should I just update the whole net package dependency ? Or just update to my commit ?
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Jorropo.
1 comment:
File src/net/http/h2_bundle.go:
Patch Set #8, Line 33:
"io/ioutil"
Patch sent: https://go-review.googlesource.com/c/net/+/401014 […]
After the x/net CL is committed you should run "go generate net/http". You'll need to "go install golang.org/x/tools/cmd/bundle" first.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
1 comment:
File src/net/http/h2_bundle.go:
Patch Set #8, Line 33:
"io/ioutil"
After the x/net CL is committed you should run "go generate net/http". […]
Done
However I had an issue with bundle where it would import "net/http" and add "http." for all type from the net package. (as if h2_bundle.go was an external package importing net/http)
Which obviously failed:
package expvar
imports net/http
imports net/http: import cycle not allowedSo FYI, I had to
sed -i "s/http\.//g" h2_bundle.goTo fix it, which seems a hack to me.
I also did a manual review of the diff to add back "http." in comments (because whatever was used previously left them in).
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Damien Neil, Robert Griesemer, Ian Lance Taylor.
Gerrit Bot uploaded patch set #9 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
This also includes updating golang.org/x/net/ for the required CL 401014.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: 4d6a26db03f48962231796e586d9fa084a4adcd5
GitHub-Pull-Request: golang/go#52340
---
M src/go.mod
M src/go.sum
M src/io/io.go
M src/io/io_test.go
M src/net/http/h2_bundle.go
M src/net/http/transfer.go
M src/vendor/golang.org/x/net/dns/dnsmessage/message.go
M src/vendor/golang.org/x/net/http/httpguts/httplex.go
M src/vendor/golang.org/x/net/http/httpproxy/proxy.go
M src/vendor/golang.org/x/net/idna/trieval.go
M src/vendor/modules.txt
11 files changed, 163 insertions(+), 69 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Patchset:
Please do the "go generate" CL as a CL all by itself, not combined with other changes. Thanks.
Are you sure you have an up to date version of the bundle program? When I run "go generate net/http" it doesn't introduce any changes to the files.
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Ian Lance Taylor, Robert Griesemer.
1 comment:
Patchset:
Please do the "go generate" CL as a CL all by itself, not combined with other changes. Thanks. […]
Done at CL 403136
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Ian Lance Taylor, Robert Griesemer.
Gerrit Bot uploaded patch set #10 to this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: a6b9af4e945a6903735a74aa185e2d1c4c2e2cef
GitHub-Pull-Request: golang/go#52340
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 73 insertions(+), 4 deletions(-)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Jorropo (Gerrit)
Attention is currently required from: Ian Lance Taylor, Robert Griesemer.
1 comment:
Patchset:
Done at CL 403136
Rebased
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Robert Griesemer.
Patch set 10:Run-TryBot +1
Ian Lance Taylor (Gerrit)
Attention is currently required from: Robert Griesemer.
Patch set 10:Run-TryBot +1Auto-Submit +1Code-Review +2
1 comment:
Patchset:
Thanks for your patience with this.
RELNOTE=yes
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.
Benny Siegert (Gerrit)
Attention is currently required from: Robert Griesemer.
Patch set 10:Code-Review +1
Gopher Robot (Gerrit)
Gopher Robot submitted this change.
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: a6b9af4e945a6903735a74aa185e2d1c4c2e2cef
GitHub-Pull-Request: golang/go#52340
Reviewed-on: https://go-review.googlesource.com/c/go/+/400236
Run-TryBot: Ian Lance Taylor <ia...@golang.org>
Run-TryBot: Ian Lance Taylor <ia...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Ian Lance Taylor <ia...@google.com>
Reviewed-by: Benny Siegert <bsie...@gmail.com>
Auto-Submit: Ian Lance Taylor <ia...@google.com>
---
M src/io/io.go
M src/io/io_test.go
M src/net/http/transfer.go
3 files changed, 80 insertions(+), 4 deletions(-)
diff --git a/src/io/io.go b/src/io/io.go
index 1ea01d5..db88125 100644
--- a/src/io/io.go
+++ b/src/io/io.go
@@ -621,7 +621,12 @@
// NopCloser returns a ReadCloser with a no-op Close method wrapping
// the provided Reader r.
+// If r implements WriterTo, the returned ReadCloser will implement WriterTo
+// by forwarding calls to r.
func NopCloser(r Reader) ReadCloser {
+ if _, ok := r.(WriterTo); ok {
+ return nopCloserWriterTo{r}
+ }
return nopCloser{r}
}
@@ -631,6 +636,16 @@
func (nopCloser) Close() error { return nil }
+type nopCloserWriterTo struct {
+ Reader
+}
+
+func (nopCloserWriterTo) Close() error { return nil }
+
+func (c nopCloserWriterTo) WriteTo(w Writer) (n int64, err error) {
+ return c.Reader.(WriterTo).WriteTo(w)
+}
+
// ReadAll reads from r until an error or EOF and returns the data it read.
// A successful call returns err == nil, not err == EOF. Because ReadAll is
// defined to read from src until EOF, it does not treat an EOF from Read
diff --git a/src/io/io_test.go b/src/io/io_test.go
index 3088460..a51a1fa 100644
--- a/src/io/io_test.go
+++ b/src/io/io_test.go
@@ -471,3 +471,24 @@
t.Errorf("Copy error: got %v, want %v", err, want)
}
}
+
+func TestNopCloserWriterToForwarding(t *testing.T) {
+ for _, tc := range [...]struct {
+ Name string
+ r Reader
+ }{
+ {"not a WriterTo", Reader(nil)},
+ {"a WriterTo", struct {
+ Reader
+ WriterTo
+ }{}},
+ } {
+ nc := NopCloser(tc.r)
+
+ _, expected := tc.r.(WriterTo)
+ _, got := nc.(WriterTo)
+ if expected != got {
+ t.Errorf("NopCloser incorrectly forwards WriterTo for %s, got %t want %t", tc.Name, got, expected)
+ }
+ }
+}
diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index d9edf8c..7bea586 100644
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -422,8 +422,8 @@
//
// This function is only intended for use in writeBody.
func (t *transferWriter) unwrapBody() io.Reader {
- if reflect.TypeOf(t.Body) == nopCloserType {
- return reflect.ValueOf(t.Body).Field(0).Interface().(io.Reader)
+ if r, ok := unwrapNopCloser(t.Body); ok {
+ return r
}
if r, ok := t.Body.(*readTrackingBody); ok {
r.didRead = true
@@ -1081,6 +1081,21 @@
}
var nopCloserType = reflect.TypeOf(io.NopCloser(nil))
+var nopCloserWriterToType = reflect.TypeOf(io.NopCloser(struct {
+ io.Reader
+ io.WriterTo
+}{}))
+
+// unwrapNopCloser return the underlying reader and true if r is a NopCloser
+// else it return false
+func unwrapNopCloser(r io.Reader) (underlyingReader io.Reader, isNopCloser bool) {
+ switch reflect.TypeOf(r) {
+ case nopCloserType, nopCloserWriterToType:
+ return reflect.ValueOf(r).Field(0).Interface().(io.Reader), true
+ default:
+ return nil, false
+ }
+}
// isKnownInMemoryReader reports whether r is a type known to not
// block on Read. Its caller uses this as an optional optimization to
@@ -1090,8 +1105,8 @@
case *bytes.Reader, *bytes.Buffer, *strings.Reader:
return true
}
- if reflect.TypeOf(r) == nopCloserType {
- return isKnownInMemoryReader(reflect.ValueOf(r).Field(0).Interface().(io.Reader))
+ if r, ok := unwrapNopCloser(r); ok {
+ return isKnownInMemoryReader(r)
}
if r, ok := r.(*readTrackingBody); ok {
return isKnownInMemoryReader(r.ReadCloser)
To view, visit change 400236. To unsubscribe, or for help writing mail filters, visit settings.