Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
rsc opened this issue Feb 23, 2022 · 9 comments
Open

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

rsc opened this issue Feb 23, 2022 · 9 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@rsc
Copy link
Contributor

rsc commented Feb 23, 2022

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.
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 23, 2022
@rsc rsc added this to the Backlog milestone Feb 23, 2022
@rsc rsc mentioned this issue Feb 23, 2022
Closed
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Feb 23, 2022
@ianlancetaylor
Copy link
Contributor

CC @randall77 @mdempsky

@randall77
Copy link
Contributor

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
Copy link

@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
Copy link

elagergren-spideroak commented Feb 26, 2022 •
edited

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
Copy link
Contributor

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
Copy link

elagergren-spideroak commented Feb 26, 2022 •
edited

@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
Copy link
Contributor

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

@elagergren-spideroak
Copy link

@randall77 yep! That's it.

@gopherbot
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@rsc @ianlancetaylor @randall77 @gopherbot @elagergren-spideroak