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

copyZeroAlloc allocates when interface uses genericWriteTo #1889

Open
lunixbochs opened this issue Oct 25, 2024 · 1 comment
Open

copyZeroAlloc allocates when interface uses genericWriteTo #1889

lunixbochs opened this issue Oct 25, 2024 · 1 comment

Comments

@lunixbochs
Copy link

lunixbochs commented Oct 25, 2024

it looks like go's io.CopyBuffer always uses the WriterTo interface if it's available, much like copyZeroAlloc

however, File and TCPConn unfortunately have a fallback in their WriterTo that calls io.Copy if sendfile isn't possible, which allocates an implicit buffer

https://github.com/golang/go/blob/15c558016088d6aaf103b4f0fd2b716a4573e5a2/src/net/net.go#L789
https://github.com/golang/go/blob/15c558016088d6aaf103b4f0fd2b716a4573e5a2/src/os/file.go#L274

this is with an os.File reaching writeBodyFixedSize

@lunixbochs
Copy link
Author

lunixbochs commented Oct 25, 2024

it doesn't look like the golang zero copy backend works with bufio.Writer at all, despite the comment here

fasthttp/http.go

Lines 2205 to 2211 in 40bdc4a

if earlyFlush {
// w buffer must be empty for triggering
// sendfile path in bufio.Writer.ReadFrom.
if err := w.Flush(); err != nil {
return err
}
}

the os.File writeTo implementation will see that the destination is not a network socket and immediately fall back to io.Copy, which allocates an implicit buffer:
https://github.com/golang/go/blob/15c558016088d6aaf103b4f0fd2b716a4573e5a2/src/os/zero_copy_linux.go#L19

func (f *File) writeTo(w io.Writer) (written int64, handled bool, err error) {
	pfd, network := getPollFDAndNetwork(w)
	// TODO(panjf2000): same as File.spliceToFile.
	if pfd == nil || !pfd.IsStream || !isUnixOrTCP(string(network)) {
		return
	}

https://github.com/golang/go/blob/15c558016088d6aaf103b4f0fd2b716a4573e5a2/src/os/zero_copy_linux.go#L163

func getPollFDAndNetwork(i any) (*poll.FD, poll.String) {
	sc, ok := i.(syscall.Conn)
	if !ok {
		return nil, ""
	}

benmcclelland pushed a commit to lunixbochs/versitygw that referenced this issue Oct 28, 2024
From versity#919, This provides the *os.File handle to io.Copy() for the
case where the full file range is requested. This prevents hiding
the *os.File type for io.Copy() optimizations.

This still requires the change to valyala/fasthttp#1889 to expose
the net.Conn similarly to enable the linux sendfile optimization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant