os: reuse buffer pool more aggressively in readdir

We can reuse the buffer pool more aggressively when reading a directory
by returning the buffer to the pool as soon as we get to the end of the
directory, rather than waiting until the the os.File is closed.

This yields a significant memory usage reduction when traversing
nested directories recursively via os.File#ReadDir (and friends),
as the file pointers tends to be closed only after the entire
traversal is done. For example, this pattern is used in os.RemoveAll.
These are the improvements observed in BenchmarkRemoveAll:

goos: linux
goarch: amd64
pkg: os
cpu: AMD EPYC 7763 64-Core Processor
            │   old.txt   │            new.txt            │
            │   sec/op    │   sec/op     vs base          │
RemoveAll-4   3.847m ± 2%   3.823m ± 1%  ~ (p=0.143 n=10)

            │   old.txt    │               new.txt                │
            │     B/op     │     B/op      vs base                │
RemoveAll-4   39.77Ki ± 2%   17.63Ki ± 1%  -55.68% (p=0.000 n=10)

            │  old.txt   │              new.txt              │
            │ allocs/op  │ allocs/op   vs base               │
RemoveAll-4   510.0 ± 0%   503.0 ± 0%  -1.37% (p=0.000 n=10)

Change-Id: I70e1037378a02f1d670ccb7b275ee55f0caa6d0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/573358
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>

diff --git a/src/os/dir_unix.go b/src/os/dir_unix.go
index 1e8d1d0a3050b483e73c655278f6d61aba2ea6c8..e14edc13dc7089ad788cf1c15f47729485c469a7 100644 (file)
--- a/src/os/dir_unix.go
+++ b/src/os/dir_unix.go
@@ -46,9 +46,11 @@ func (f *File) readdir(n int, mode readdirMode) (names []string, dirents []DirEn
        // If this file has no dirinfo, create one.
        if f.dirinfo == nil {
                f.dirinfo = new(dirInfo)
-               f.dirinfo.buf = dirBufPool.Get().(*[]byte)
        }
        d := f.dirinfo
+       if d.buf == nil {
+               f.dirinfo.buf = dirBufPool.Get().(*[]byte)
+       }
 
        // Change the meaning of n for the implementation below.
        //
@@ -74,6 +76,9 @@ func (f *File) readdir(n int, mode readdirMode) (names []string, dirents []DirEn
                                return names, dirents, infos, &PathError{Op: "readdirent", Path: f.name, Err: errno}
                        }
                        if d.nbuf <= 0 {
+                               // Optimization: we can return the buffer to the pool, there is nothing else to read.
+                               dirBufPool.Put(d.buf)
+                               d.buf = nil
                                break // EOF
                        }
                }

diff --git a/src/os/dir_windows.go b/src/os/dir_windows.go
index 4485dffdb184e164db696a34f57b93918c92ac67..5ba1d4640a5d98c2d715a85382679698eee9b184 100644 (file)
--- a/src/os/dir_windows.go
+++ b/src/os/dir_windows.go
@@ -72,7 +72,6 @@ func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []Di
                        return
                }
                file.dirinfo = new(dirInfo)
-               file.dirinfo.buf = dirBufPool.Get().(*[]byte)
                file.dirinfo.vol = vol
                if allowReadDirFileID && flags&windows.FILE_SUPPORTS_OPEN_BY_FILE_ID != 0 {
                        file.dirinfo.class = windows.FileIdBothDirectoryRestartInfo
@@ -94,6 +93,9 @@ func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []Di
                }
        }
        d := file.dirinfo
+       if d.buf == nil {
+               d.buf = dirBufPool.Get().(*[]byte)
+       }
        wantAll := n <= 0
        if wantAll {
                n = -1
@@ -105,6 +107,9 @@ func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []Di
                        runtime.KeepAlive(file)
                        if err != nil {
                                if err == syscall.ERROR_NO_MORE_FILES {
+                                       // Optimization: we can return the buffer to the pool, there is nothing else to read.
+                                       dirBufPool.Put(d.buf)
+                                       d.buf = nil
                                        break
                                }
                                if err == syscall.ERROR_FILE_NOT_FOUND &&

diff --git a/src/os/removeall_test.go b/src/os/removeall_test.go
index c0b2dd6505608c4f39d1b4827cf771f1b04dfac5..6aca98021fb393e64760f16329fd77590b1ea5b5 100644 (file)
--- a/src/os/removeall_test.go
+++ b/src/os/removeall_test.go
@@ -504,3 +504,20 @@ func TestRemoveAllNoFcntl(t *testing.T) {
                t.Errorf("found %d fcntl calls, want < 100", got)
        }
 }
+
+func BenchmarkRemoveAll(b *testing.B) {
+       tmpDir := filepath.Join(b.TempDir(), "target")
+       b.ReportAllocs()
+       b.ResetTimer()
+       for i := 0; i < b.N; i++ {
+               b.StopTimer()
+               err := CopyFS(tmpDir, DirFS("."))
+               if err != nil {
+                       b.Fatal(err)
+               }
+               b.StartTimer()
+               if err := RemoveAll(tmpDir); err != nil {
+                       b.Fatal(err)
+               }
+       }
+}