golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.98k stars 17.67k forks source link

cmd/compile: better escape analysis for net.SyscallConn #51334

Open rsc opened 2 years ago

rsc commented 2 years ago

Programs using net.SyscallConn look like:

sc := c.(*net.TCPConn).SyscallConn()
var werr error
err = sc.Read(func(fd uintptr) bool {
    written, werr = poll.SendFile(&c.pfd, int(fd), remain)
    return true
})
if err == nil {
    err = werr
}

Although sc is an interface value of type net.SyscallConn, the compiler could know the underlying type, because there's only one that net.TCPConn.SyscallConn returns. It should therefore be able to see that sc.Read does not escape its argument, so that the closure can be stack-allocated. But today the closure is heap-allocated. It would be good to make the compiler smart enough to avoid the allocation.

See also #51170, which proposed an API change, but a smarter compiler with no new API would be better.

ianlancetaylor commented 2 years ago

CC @randall77 @mdempsky

randall77 commented 2 years ago

We already have a devirtualization pass, but it can only handle pretty simple cases. The case relevant here is that we can devirtualize, but only if SyscallConn is inlined.

package main

type T struct {
}

func (t T) foo(b []byte) {
}

type I interface {
    foo([]byte)
}

func f() I {
    return T{}
}

func main() {
    b := make([]byte, 32)
    f().foo(b)
}

The foo call in main is called directly, not with an interface call. Escape analysis also sees that, so the byte slice allocated in main can be stack allocated.

If you mark f as //go:noinline, then that analysis falls apart and the byte slice allocated in main must be heap allocated.

All that said, this issue may be fixable just by rejiggering SyscallConn to allocate its result, call a helper to do the actual work, and then return that allocated result. Such a SyscallConn would be inlineable, and might just fix this issue without any compiler changes.

Not that it wouldn't be helpful to make the compiler better, but FYI in case you're looking for something nearer term.

elagergren-spideroak commented 2 years ago

@randall77 note that (*os.File).SyscallConn, which is used in the original issue, is inlineable (using tip).

./sendfile_linux.go:21:6: cannot inline sendFile: function too complex: cost 277 exceeds budget 80
./sendfile_linux.go:36:26: inlining call to os.(*File).SyscallConn
./sendfile_linux.go:53:34: inlining call to wrapSyscallError
./sendfile_linux.go:53:34: inlining call to os.NewSyscallError
./sendfile_linux.go:42:16: can inline sendFile.func1 with cost 70 as: func(uintptr) bool { written, werr = poll.SendFile(&c.pfd, int(fd), remain); return true }

However, everything still escapes

./sendfile_linux.go:42:16: func literal escapes to heap:
./sendfile_linux.go:42:16:   flow: {heap} = &{storage for func literal}:
./sendfile_linux.go:42:16:     from func literal (spill) at ./sendfile_linux.go:42:16
./sendfile_linux.go:42:16:     from sc.Read(func literal) (call parameter) at ./sendfile_linux.go:42:15
./sendfile_linux.go:21:15: sendFile capturing by value: c (addr=false assign=false width=8)
./sendfile_linux.go:22:6: sendFile capturing by value: remain (addr=false assign=false width=8)
./sendfile_linux.go:21:39: sendFile capturing by ref: written (addr=false assign=true width=8)
./sendfile_linux.go:21:39: written escapes to heap:
./sendfile_linux.go:21:39:   flow: {storage for func literal} = &written:
./sendfile_linux.go:21:39:     from written (captured by a closure) at ./sendfile_linux.go:43:3
./sendfile_linux.go:21:39:     from written (reference) at ./sendfile_linux.go:43:3
./sendfile_linux.go:41:6: sendFile capturing by ref: werr (addr=false assign=true width=16)
./sendfile_linux.go:41:6: werr escapes to heap:
./sendfile_linux.go:41:6:   flow: {storage for func literal} = &werr:
./sendfile_linux.go:41:6:     from werr (captured by a closure) at ./sendfile_linux.go:43:12
./sendfile_linux.go:41:6:     from werr (reference) at ./sendfile_linux.go:43:12
./sendfile_linux.go:21:15: parameter c leaks to {heap} with derefs=0:
./sendfile_linux.go:21:15:   flow: {heap} = c:
./sendfile_linux.go:21:15:     from c.pfd (dot of pointer) at ./sendfile_linux.go:43:35
./sendfile_linux.go:21:15:     from &c.pfd (address-of) at ./sendfile_linux.go:43:33
./sendfile_linux.go:21:15:     from poll.SendFile(&c.pfd, int(fd), remain) (call parameter) at ./sendfile_linux.go:43:32
./sendfile_linux.go:36:26: &os.rawConn{...} escapes to heap:
./sendfile_linux.go:36:26:   flow: ~R0 = &{storage for &os.rawConn{...}}:
./sendfile_linux.go:36:26:     from &os.rawConn{...} (spill) at ./sendfile_linux.go:36:26
./sendfile_linux.go:36:26:     from syscall.RawConn(&os.rawConn{...}) (interface-converted) at ./sendfile_linux.go:36:26
./sendfile_linux.go:36:26:     from ~R0, ~R1 = syscall.RawConn(&os.rawConn{...}), nil (assign-pair) at ./sendfile_linux.go:36:26
./sendfile_linux.go:36:26:   flow: sc = ~R0:
./sendfile_linux.go:36:26:   flow: {heap} = sc:
./sendfile_linux.go:36:26:     from sc.Read(func literal) (call parameter) at ./sendfile_linux.go:42:15
./sendfile_linux.go:21:15: parameter c leaks to {storage for func literal} with derefs=0:
./sendfile_linux.go:21:15:   flow: {storage for func literal} = c:
./sendfile_linux.go:21:15:     from c (captured by a closure) at ./sendfile_linux.go:43:34
./sendfile_linux.go:21:25: parameter r leaks to {storage for &os.rawConn{...}} with derefs=0:
./sendfile_linux.go:21:25:   flow: f = r:
./sendfile_linux.go:21:25:     from r.(*os.File) (dot) at ./sendfile_linux.go:31:12
./sendfile_linux.go:21:25:     from f, ok := r.(*os.File) (assign-pair-dot-type) at ./sendfile_linux.go:31:8
./sendfile_linux.go:21:25:   flow: os.f = f:
./sendfile_linux.go:21:25:     from os.f := f (assign-pair) at ./sendfile_linux.go:36:26
./sendfile_linux.go:21:25:   flow: {storage for &os.rawConn{...}} = os.f:
./sendfile_linux.go:21:25:     from os.rawConn{...} (struct literal element) at ./sendfile_linux.go:36:26
./sendfile_linux.go:53:34: &os.SyscallError{...} escapes to heap:
./sendfile_linux.go:53:34:   flow: ~R0 = &{storage for &os.SyscallError{...}}:
./sendfile_linux.go:53:34:     from &os.SyscallError{...} (spill) at ./sendfile_linux.go:53:34
./sendfile_linux.go:53:34:     from error(&os.SyscallError{...}) (interface-converted) at ./sendfile_linux.go:53:34
./sendfile_linux.go:53:34:     from ~R0 = error(&os.SyscallError{...}) (assign-pair) at ./sendfile_linux.go:53:34
./sendfile_linux.go:53:34:   flow: err = ~R0:
./sendfile_linux.go:53:34:     from err = ~R0 (assign) at ./sendfile_linux.go:53:34
./sendfile_linux.go:53:34:   flow: ~R0 = err:
./sendfile_linux.go:53:34:     from ~R0 = err (assign-pair) at ./sendfile_linux.go:53:34
./sendfile_linux.go:53:34:   flow: err = ~R0:
./sendfile_linux.go:53:34:     from return written, ~R0, written > 0 (return) at ./sendfile_linux.go:53:2
./sendfile_linux.go:21:15: leaking param: c
./sendfile_linux.go:21:25: leaking param: r
./sendfile_linux.go:21:39: moved to heap: written
./sendfile_linux.go:41:6: moved to heap: werr
./sendfile_linux.go:36:26: &os.rawConn{...} escapes to heap
./sendfile_linux.go:42:16: func literal escapes to heap
./sendfile_linux.go:53:34: &os.SyscallError{...} escapes to heap
elagergren-spideroak commented 2 years ago

When I add type RawConn struct { *rawConn } to the os package and add an assertion inside sendFile, the compiler can prove that Read doesn't escape. (Obviously this isn't the correct solution.)

    sc, err := f.SyscallConn()
    if err != nil {
        return 0, nil, false
    }
    rc := sc.(*os.RawConn)

    var werr error
    err = rc.Read(func(fd uintptr) bool {
        written, werr = poll.SendFile(&c.pfd, int(fd), remain)
        return true
    })
randall77 commented 2 years ago

In OS, SyscallConn is inlineable but the allocation is one level lower, in newRawConn. Both would need to inline (maybe they do? newRawConn is simple), and the analysis would have to handle 2 levels of inlining (maybe it doesn't?).

elagergren-spideroak commented 2 years ago

@randall77 newRawConn is simply

func newRawConn(file *File) (*rawConn, error) {
    return &rawConn{file: file}, nil
}

But even manually manual inlining the allocation does not work:

func (f *File) SyscallConn() (syscall.RawConn, error) {
    return &rawConn{file: f}, nil
}
randall77 commented 2 years ago

Maybe it is the 2 results. The analyzer can only handle single results.

elagergren-spideroak commented 2 years ago

@randall77 yep! That's it.

gopherbot commented 8 months ago

Change https://go.dev/cl/567898 mentions this issue: cmd/compile: handle devirtualization of multiple params