ByteStream consumer can write to interface{}

[fredbi]

ByteStream consumer can write to interface{}

• fix(ByteStreamConsumer): may now write into an interface which
  underlying type is []byte or string.

• feat(ByteStreamConsumer): added support to io.ReaderFrom, preferred
  over io.Writer if available

• feat(ByteStreamProducer): added support to io.WriterTo, preferred
  over io.Reader if available

• refact(ByteStreamProducer): removed redundant case "string" and preferred
  the more general reflected case (supports aliased strings)

• test: refactored ByteStream tests

• test: added benchmark for bytestream.Consume

• fixes ByteStreamConsumer can't write to interface{} #167

Signed-off-by: Frederic BIDON [email protected]

[fredbi]

cc: @danny-cheung

[fredbi]

@casualjim @youyuanwu I am interested by your feedback on this approach, as it doesn't make sense to adopt it for just the Bytestream Consumer. Please let me know what are your thoughts.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (248b38c) 80.83% compared to head (d9f89fe) 80.99%.

❗ Current head d9f89fe differs from pull request most recent head 0a0b04d. Consider uploading reports for the commit 0a0b04d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   80.83%   80.99%   +0.15%     
==========================================
  Files          44       44              
  Lines        3366     3394      +28     
==========================================
+ Hits         2721     2749      +28     
  Misses        535      535              
  Partials      110      110              

Flag	Coverage Δ
oldstable	80.99% <100.00%> (+0.15%)	⬆️
stable	80.99% <100.00%> (+0.15%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fredbi]

@casualjim I'd like to add a little more test work on that one. I'd like to benchmark a bit to figure out the actual savings with not entering the "reflected" code path. If this is proves to be a sound pattern, I'll generalize in a follow-up pattern, e.g. fixing #263

[fredbi]

go test -v -bench Consumer -run Bench -benchtime 30s -memprofile mem.out
goos: linux
goarch: amd64
pkg: github.com/go-openapi/runtime
cpu: AMD Ryzen 7 5800X 8-Core Processor             
BenchmarkByteStreamConsumer
BenchmarkByteStreamConsumer/with_writer
BenchmarkByteStreamConsumer/with_writer-16         	1000000000	        27.72 ns/op	       0 B/op	       0 allocs/op
BenchmarkByteStreamConsumer/with_BinaryUnmarshal
BenchmarkByteStreamConsumer/with_BinaryUnmarshal-16         	48124560	       819.5 ns/op	    3584 B/op	       3 allocs/op
BenchmarkByteStreamConsumer/with_string
BenchmarkByteStreamConsumer/with_string-16                  	34500576	      1102 ns/op	    4608 B/op	       4 allocs/op
BenchmarkByteStreamConsumer/with_[]byte
BenchmarkByteStreamConsumer/with_[]byte-16                  	44077273	       882.6 ns/op	    3584 B/op	       3 allocs/op
BenchmarkByteStreamConsumer/with_aliased_string
BenchmarkByteStreamConsumer/with_aliased_string-16          	33367702	      1120 ns/op	    4608 B/op	       4 allocs/op
BenchmarkByteStreamConsumer/with_aliased_[]byte
BenchmarkByteStreamConsumer/with_aliased_[]byte-16          	38591841	       916.2 ns/op	    3584 B/op	       3 allocs/op
PASS
ok  	github.com/go-openapi/runtime	224.368s

[fredbi]

@casualjim the benchmark demonstrates that I was wrong ...

There is actually no material benefit from favoring type a switch over reflect (when done correctly). The benchmark shows that the reflection path is actually quite efficient and that the reflect.Value is not allocated on the heap.

Therefore, I am going to simplify this and remove the redundant cases.