From 051e99292f0b3551936e841e6aa6484ec66dd906 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Mon, 25 Mar 2024 12:19:47 +0100 Subject: [PATCH] os: reuse buffer pool more aggressively in readdir MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Ian Lance Taylor Reviewed-by: Emmanuel Odeke Reviewed-by: Matthew Dempsky Auto-Submit: Emmanuel Odeke --- src/os/dir_unix.go | 7 ++++++- src/os/dir_windows.go | 7 ++++++- src/os/removeall_test.go | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/os/dir_unix.go b/src/os/dir_unix.go index 1e8d1d0a30..e14edc13dc 100644 --- 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 4485dffdb1..5ba1d4640a 100644 --- 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 c0b2dd6505..6aca98021f 100644 --- 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) + } + } +} -- 2.48.1