syscall: add array of Errno errors?

[josharian]

Running os/exec's new BenchmarkExecEcho on my machine, 8% of allocations are from internal/poll/fd_poll_runtime.go:45: return syscall.Errno(errno). This allocates when we convert from syscall.Errno to an error. (For those who are curious, the errno is 22, EINVAL, indicating that the file is not pollable. We end up noting only that the error is non-nil, at internal/poll/fd_unix.go:64, and then dropping it, at os/file_unix.go:157.)

There aren't many errnos. We could avoid this allocation, and others, by adding an array of all expected Errnos as errors in package syscall, and adding an accessor function to return them. Something like:

var errnoErrors = [...]error{nil, Errno(1), Errno(2), ... }

func ErrnoError(e int) error {
  if e >= 0 && e < len(errnoErrors) {
    return errnoErrors[e]
  }
  return Errno(e)
}

Note that errnoErrors would be generated by a script, and would be treated as static data by the compiler and linker, so the impact would be only on binary size (not compilation time or process startup).

Package syscall is frozen and sensitive, so I wanted to ask before doing the work.

cc @bradfitz @ianlancetaylor

[bradfitz]

Sounds fine. We've done that elsewhere in std already.

[bradfitz]

For instance:

• https://go-review.googlesource.com/c/go/+/6701/
• https://go-review.googlesource.com/c/go/+/8192/
• https://go-review.googlesource.com/c/go/+/28484/

[mvdan]

@josharian I'm curious; is there a reason you're looking into the performance of os/exec recently?

[josharian]

In some circumstances it was showed up in go-fuzz pprof output. See also dvyukov/go-fuzz#215.

[josharian]

I started on this and then started to wonder whether we should do this trick in the compiler for small ints in interfaces instead, like we do with single-byte values. We have talked about that in the past and worried about binary size and weird perf cliffs. But this change would add more to binary size, and almost everyone uses syscall, and doing it in the compiler would benefit other code.

[josharian]

(Note to self; on phone.)

A narrower fix would be to add a ByteErrno, applicable only for byte-sized errnos, that delegates to syscall.Errno for errors strings.

[bradfitz]

A narrower fix would be to add a ByteErrno

No adding exported stuff to syscall, though. And we can't change the type on people anyway.

[bradfitz]

Btw, we already have:

https://github.com/golang/go/blob/8f9902d/src/syscall/syscall_unix.go#L139

// errnoErr returns common boxed Errno values, to prevent
// allocations at runtime.
func errnoErr(e Errno) error {
	switch e {
	case 0:
		return nil
	case EAGAIN:
		return errEAGAIN
	case EINVAL:
		return errEINVAL
	case ENOENT:
		return errENOENT
	}
	return e
}

[bradfitz]

... feel free to add to that. Or change its implementation to the array thing.

[josharian]

No adding exported stuff to syscall, though. And we can't change the type on people anyway.

I was thinking of it just for this particular use, which never makes it to the outside world.

errnoErr is perfect but unexported and in the wrong package. I suppose I could export it and move it to some internal package.

[bradfitz]

errnoErr is perfect but unexported and in the wrong package. I suppose I could export it and move it to some internal package.

SGTM

[gopherbot]

Change https://golang.org/cl/164971 mentions this issue: internal/poll: copy and use errnoErr to avoid allocations