Speed of io.Reader

[Dave MacFarlane]

I'm not sure if this is a question for golang-dev or go-nuts, but I'm
encountering something that I'm not sure if I should file as a bug in
Go or if it's just a suck-it-up-princess situation...

I started writing a simple login* shell in Go for fun this weekend. I
tried adding background process/job management support tonight, which
means I need to intercept the Stdin and Stdout of things I exec so
that I can send SIGTTIN and/or SIGTTOU if they try and read or write
anything from the background, but the performance drops noticably in
the non-backgrounded case if I change process.Stdin to a simple
wrapper around os.Stdin instead of directly using os.Stdin.

I wrote some benchmarks for a wrapper which does nothing but return
os.Stdin.Read, and the performance is between 75-100% worse when
reading from the wrapper around os.Stdin vs directly reading os.Stdin.
I'd expect some overhead, but halving the performance seems excessive.
Should I file a bug report over this, or is that the expected level of
overhead of using an io.Reader? (1.7.3 vs tip doesn't seem to make
much difference.)

➜ test ~/go/bin/go test -bench .
BenchmarkReadStdin-4 1000000 1261 ns/op
BenchmarkReadStdinWrapper-4 1000000 2252 ns/op
BenchmarkReadStdinWrapperP-4 1000000 2102 ns/op


The benchmark code is just:

package main

import (
"os"
"testing"
)

type WrapperStruct struct{}

func (w WrapperStruct) Read(b []byte) (int, error) {
return os.Stdin.Read(b)
}

type WrapperStructP struct{}

func (w *WrapperStructP) Read(b []byte) (int, error) {
return os.Stdin.Read(b)
}

func BenchmarkReadStdin(b *testing.B) {
go func() {
for {
os.Stdin.Write([]byte{'\n'})
}
}()
for i := 0; i < b.N; i++ {
b := make([]byte, 1)
os.Stdin.Read(b)
}
}

func BenchmarkReadStdinWrapper(b *testing.B) {
go func() {
for {
os.Stdin.Write([]byte{'\n'})
}
}()
w := WrapperStruct{}
for i := 0; i < b.N; i++ {
b := make([]byte, 1)
w.Read(b)
}
}
func BenchmarkReadStdinWrapperP(b *testing.B) {
go func() {
for {
os.Stdin.Write([]byte{'\n'})
}
}()
w := &WrapperStructP{}
for i := 0; i < b.N; i++ {
b := make([]byte, 1)
w.Read(b)
}
}


* Can not yet be used as a login shell.
--
- Dave

[Brad Fitzpatrick]

This looks like an allocation benchmark more than anything.

My guess is that this allocating tons of 1 byte objects for both reading and writing, except in the case of the concrete type, where the compiler can prove to itself that the memory doesn't escape, and then stack-allocate it instead as an optimization. Using the interface prevents the compiler from knowing for sure what'll escape.

But the real bug is that you're allocating like crazy in a loop. Fix that and I suspect the performance difference almost completely disappears.

--
- Dave

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[minux]

There are multiple problems with the benchmark code.

1. It's leaking goroutines at each call of Benchmark*.

2. it's allocating a new buffer at each round of i := 0 to b.N loop, which

means it benchmarks the memory allocator as much as anything.

[Dave MacFarlane]

Thanks, fixing those fixed the benchmarks, but unfortunately that just
makes it more difficult to demonstrate the problem objectively.

The real issue is that, from a user's perspective this code is zippy:

cmd := exec.Command("more", "foo.go")
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

if err := cmd.Start(); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
}
if err := cmd.Wait(); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
}


while this code (with the same WrapperStruct{} from above) is
noticeably slow and clunky.

cmd := exec.Command("more", "foo.go")
cmd.Stdin = WrapperStruct{}
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

if err := cmd.Start(); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
}
if err := cmd.Wait(); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
}

But I don't know of a good way to benchmark reading user input for an
interactive exec.Cmd.

--
- Dave

[Peter Mrekaj]

First, as Brad has already mentioned, your benchmark includes allocation (and also spawning Goutine). Second, check the error returned by os.Stdin.Write in your benchmark and you'll be surprised. Third, this modification will give you more reliable results (if benching wrapped against non-wrapped):

type WrapperStruct struct{
   reader io.Reader

}

func (w WrapperStruct) Read(b []byte) (int, error) {

   return w.reader.Read(b)
}

type WrapperStructPtr struct{
   reader io.Reader
}

func (w *WrapperStructPtr) Read(b []byte) (int, error) {
   return w.reader.Read(b)
}

func bench(b *testing.B, r io.Reader, w io.Writer) {
   done := make(chan struct{})
   newline := []byte{'\n'}
   go func() {
      for {
         select {
         case <-done:
            return
         default:
            _, err := w.Write(newline)
            if err != nil {
               b.Errorf("couldn't write to pipe: %v", err)
            }
         }
      }
   }()
   buf := make([]byte, 1)
   b.ResetTimer()

   for i := 0; i < b.N; i++ {

      _, err := r.Read(buf)
      if err != nil {
         b.Errorf("couldn't read from pipe: %v", err)
      }
   }
   done <- struct{}{}
}

func BenchmarkNoWrapper(b *testing.B)        {
   r, w := io.Pipe()
   defer w.Close()
   bench(b, r, w)
}
func BenchmarkWrapperStruct(b *testing.B) {
   r, w := io.Pipe()
   defer w.Close()
   bench(b, WrapperStruct{r}, w)
}
func BenchmarkWapperStructPtr(b *testing.B) {
   r, w := io.Pipe()
   defer w.Close()
   bench(b, &WrapperStructPtr{r}, w)
}

Although I'm still not quite sure what you're trying to bench (the stdin or the wrappers)!

[roger peppe]

On 14 December 2016 at 03:23, Dave MacFarlane <dri...@gmail.com> wrote:
> I tried adding background process/job management support tonight, which
> means I need to intercept the Stdin and Stdout of things I exec so
> that I can send SIGTTIN and/or SIGTTOU if they try and read or write
> anything from the background

I don't think you can do what you want like this. You can't tell
whether a process uses its
stdin or stdout this way because the process isn't using the reader
directly.

To demonstrate: https://play.golang.org/p/DMMTyETi53

Even though sleep(1) doesn't read stdin, we see that c.Stdin is read from.
That's because os/exec is clever - when Cmd.Stdin isn't a *os.File,
it passes a pipe to the process and runs io.Copy(pipe, c.Stdin)
to make the reader data available to the process.

If you really want to implement job control (personally I'd question
its necessity, but then I use a shell without job control) then
I think you have to put the process you've started into a background
process group, but we're descending into Unix arcana here
and I refuse to be distracted further :)

BTW I think this thread is more appropriate to golang-nuts than golang-dev.

cheers,
rog.

[Dave MacFarlane]

On Wed, Dec 14, 2016 at 7:47 AM, Peter Mrekaj <mrek...@gmail.com> wrote:
> Although I'm still not quite sure what you're trying to bench (the stdin or
> the wrappers)!

Neither, I'm trying to figure out why the performance of user
io/suffers so much in the
two exec commands in my second mail, and benchmarks of the only part
that's different
seemed to be the best way to do that.

.. but roger peppe's explanation of os/exec being clever with io.Copy
probably explains
the overhead (even if it's still more extreme than I expect), and I'll
have to figure out another
way to do it.. but at this point I agree with him that it's a
discussion for go-nuts, not go-dev.

- Dave

[tacod...@gmail.com]

Additionally the Read function of the wrapper type cannot be inlined because it calls another function, namely os.Stdin.Read. This is currently restricted by the compiler when evaluating which functions can be inlined. I'm not sure if it adds so much overhead as you currently measure though.

Op woensdag 14 december 2016 17:19:04 UTC+1 schreef Dave MacFarlane:

[roger peppe]

On 15 December 2016 at 12:44, <tacod...@gmail.com> wrote:
> Additionally the Read function of the wrapper type cannot be inlined because
> it calls another function, namely os.Stdin.Read. This is currently
> restricted by the compiler when evaluating which functions can be inlined.
> I'm not sure if it adds so much overhead as you currently measure though.

This would never be inlined anyway as it's being called through an interface.

>
> Op woensdag 14 december 2016 17:19:04 UTC+1 schreef Dave MacFarlane:
>>
>> On Wed, Dec 14, 2016 at 7:47 AM, Peter Mrekaj <mrek...@gmail.com> wrote:
>> > Although I'm still not quite sure what you're trying to bench (the stdin
>> > or
>> > the wrappers)!
>>
>> Neither, I'm trying to figure out why the performance of user
>> io/suffers so much in the
>> two exec commands in my second mail, and benchmarks of the only part
>> that's different
>> seemed to be the best way to do that.
>>
>> .. but roger peppe's explanation of os/exec being clever with io.Copy
>> probably explains
>> the overhead (even if it's still more extreme than I expect), and I'll
>> have to figure out another
>> way to do it.. but at this point I agree with him that it's a
>> discussion for go-nuts, not go-dev.
>>
>> - Dave
>

> --
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to golang-dev+...@googlegroups.com.