net,internal/poll: mark it as handled even if sendfile(2) succeeded with 0 bytes...

CL 415834 fixed #53658 and somehow it only fixed it on Linux,
sendfile can also succeed with 0 bytes sent on other platforms
according to their manuals, this CL will finish the work that
CL 415834 left out on other platforms.

goos: darwin
goarch: arm64
pkg: net
                     │     old     │                new                 │
                     │   sec/op    │   sec/op     vs base               │
SendfileZeroBytes-10   7.563µ ± 5%   7.184µ ± 6%  -5.01% (p=0.009 n=10)

                     │     old     │                new                 │
                     │    B/op     │    B/op     vs base                │
SendfileZeroBytes-10   3562.5 ± 7%   590.0 ± 2%  -83.44% (p=0.000 n=10)

                     │    old    │             new              │
                     │ allocs/op │ allocs/op   vs base          │
SendfileZeroBytes-10   0.00 ± 0%   11.00 ± 0%  ? (p=0.000 n=10)

[1] https://man.freebsd.org/cgi/man.cgi?sendfile(2)
[2] https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/sendfile.2.html
[3] https://man.dragonflybsd.org/?command=sendfile&section=2
[4] https://docs.oracle.com/cd/E88353_01/html/E37843/sendfile-3c.html

Change-Id: I55832487595ee8e0f44f367cf2a3a1d827ba590d
Reviewed-on: https://go-review.googlesource.com/c/go/+/536455
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>

diff --git a/src/internal/poll/sendfile_bsd.go b/src/internal/poll/sendfile_bsd.go
index 89315a8c67352e988a5b5b6197f2cd1cafe3dacd..0f55cad73dc3ee7151cb8c5cc5b9addbacf573a9 100644 (file)
--- a/src/internal/poll/sendfile_bsd.go
+++ b/src/internal/poll/sendfile_bsd.go
@@ -13,18 +13,21 @@ import "syscall"
 const maxSendfileSize int = 4 << 20
 
 // SendFile wraps the sendfile system call.
-func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {
+func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error, bool) {
        if err := dstFD.writeLock(); err != nil {
-               return 0, err
+               return 0, err, false
        }
        defer dstFD.writeUnlock()
        if err := dstFD.pd.prepareWrite(dstFD.isFile); err != nil {
-               return 0, err
+               return 0, err, false
        }
 
        dst := dstFD.Sysfd
-       var written int64
-       var err error
+       var (
+               written int64
+               err     error
+               handled = true
+       )
        for remain > 0 {
                n := maxSendfileSize
                if int64(n) > remain {
@@ -52,8 +55,9 @@ func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {
                        // support) and syscall.EINVAL (fd types which
                        // don't implement sendfile)
                        err = err1
+                       handled = false
                        break
                }
        }
-       return written, err
+       return written, err, handled
 }

diff --git a/src/internal/poll/sendfile_solaris.go b/src/internal/poll/sendfile_solaris.go
index 7ae18f4b1a661cfa4b4c4b662eb03cff16a3bfc5..f9f685c64a6459da5f2797d13637671d6deffb07 100644 (file)
--- a/src/internal/poll/sendfile_solaris.go
+++ b/src/internal/poll/sendfile_solaris.go
@@ -16,18 +16,21 @@ import "syscall"
 const maxSendfileSize int = 4 << 20
 
 // SendFile wraps the sendfile system call.
-func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {
+func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error, bool) {
        if err := dstFD.writeLock(); err != nil {
-               return 0, err
+               return 0, err, false
        }
        defer dstFD.writeUnlock()
        if err := dstFD.pd.prepareWrite(dstFD.isFile); err != nil {
-               return 0, err
+               return 0, err, false
        }
 
        dst := dstFD.Sysfd
-       var written int64
-       var err error
+       var (
+               written int64
+               err     error
+               handled = true
+       )
        for remain > 0 {
                n := maxSendfileSize
                if int64(n) > remain {
@@ -59,8 +62,9 @@ func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {
                        // support) and syscall.EINVAL (fd types which
                        // don't implement sendfile)
                        err = err1
+                       handled = false
                        break
                }
        }
-       return written, err
+       return written, err, handled
 }

diff --git a/src/net/sendfile_test.go b/src/net/sendfile_test.go
index 997a0ed01f106dda68b1ca14e70b1347d894d091..4cba1ed2b1620a9c041d88af2949138a59da8803 100644 (file)
--- a/src/net/sendfile_test.go
+++ b/src/net/sendfile_test.go
@@ -6,6 +6,7 @@ package net
 
 import (
        "bytes"
+       "context"
        "crypto/sha256"
        "encoding/hex"
        "errors"
@@ -360,3 +361,88 @@ func TestSendfileOnWriteTimeoutExceeded(t *testing.T) {
                t.Fatal(err)
        }
 }
+
+func BenchmarkSendfileZeroBytes(b *testing.B) {
+       var (
+               wg          sync.WaitGroup
+               ctx, cancel = context.WithCancel(context.Background())
+       )
+
+       defer wg.Wait()
+
+       ln := newLocalListener(b, "tcp")
+       defer ln.Close()
+
+       tempFile, err := os.CreateTemp(b.TempDir(), "test.txt")
+       if err != nil {
+               b.Fatalf("failed to create temp file: %v", err)
+       }
+       defer tempFile.Close()
+
+       fileName := tempFile.Name()
+
+       dataSize := b.N
+       wg.Add(1)
+       go func(f *os.File) {
+               defer wg.Done()
+
+               for i := 0; i < dataSize; i++ {
+                       if _, err := f.Write([]byte{1}); err != nil {
+                               b.Errorf("failed to write: %v", err)
+                               return
+                       }
+                       if i%1000 == 0 {
+                               f.Sync()
+                       }
+               }
+       }(tempFile)
+
+       b.ResetTimer()
+       b.ReportAllocs()
+
+       wg.Add(1)
+       go func(ln Listener, fileName string) {
+               defer wg.Done()
+
+               conn, err := ln.Accept()
+               if err != nil {
+                       b.Errorf("failed to accept: %v", err)
+                       return
+               }
+               defer conn.Close()
+
+               f, err := os.OpenFile(fileName, os.O_RDONLY, 0660)
+               if err != nil {
+                       b.Errorf("failed to open file: %v", err)
+                       return
+               }
+               defer f.Close()
+
+               for {
+                       if ctx.Err() != nil {
+                               return
+                       }
+
+                       if _, err := io.Copy(conn, f); err != nil {
+                               b.Errorf("failed to copy: %v", err)
+                               return
+                       }
+               }
+       }(ln, fileName)
+
+       conn, err := Dial("tcp", ln.Addr().String())
+       if err != nil {
+               b.Fatalf("failed to dial: %v", err)
+       }
+       defer conn.Close()
+
+       n, err := io.CopyN(io.Discard, conn, int64(dataSize))
+       if err != nil {
+               b.Fatalf("failed to copy: %v", err)
+       }
+       if n != int64(dataSize) {
+               b.Fatalf("expected %d copied bytes, but got %d", dataSize, n)
+       }
+
+       cancel()
+}

diff --git a/src/net/sendfile_unix_alt.go b/src/net/sendfile_unix_alt.go
index b86771721ee2d704f1db4f652688a99c56902b4d..5cb65ee7670c49f6c4ae1bab2ca5cda209042156 100644 (file)
--- a/src/net/sendfile_unix_alt.go
+++ b/src/net/sendfile_unix_alt.go
@@ -15,8 +15,8 @@ import (
 // sendFile copies the contents of r to c using the sendfile
 // system call to minimize copies.
 //
-// if handled == true, sendFile returns the number of bytes copied and any
-// non-EOF error.
+// if handled == true, sendFile returns the number (potentially zero) of bytes
+// copied and any non-EOF error.
 //
 // if handled == false, sendFile performed no work.
 func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) {
@@ -65,7 +65,7 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) {
 
        var werr error
        err = sc.Read(func(fd uintptr) bool {
-               written, werr = poll.SendFile(&c.pfd, int(fd), pos, remain)
+               written, werr, handled = poll.SendFile(&c.pfd, int(fd), pos, remain)
                return true
        })
        if err == nil {
@@ -78,8 +78,8 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) {
 
        _, err1 := f.Seek(written, io.SeekCurrent)
        if err1 != nil && err == nil {
-               return written, err1, written > 0
+               return written, err1, handled
        }
 
-       return written, wrapSyscallError("sendfile", err), written > 0
+       return written, wrapSyscallError("sendfile", err), handled
 }