Skip to content

dirent: no longer cache the results of Stat and Info #71

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.

Sign up for GitHub

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

Draft
charlievieth wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

dirent: no longer cache the results of Stat and Info #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 11 additions & 36 deletions dirent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,51 +3,26 @@ package fastwalk
import (
"io/fs"
"os"
"sync"
"sync/atomic"
"syscall"
"unsafe"
)

type fileInfo struct {
once sync.Once
fs.FileInfo
err error
}

func loadFileInfo(pinfo **fileInfo) *fileInfo {
ptr := (*unsafe.Pointer)(unsafe.Pointer(pinfo))
fi := (*fileInfo)(atomic.LoadPointer(ptr))
if fi == nil {
fi = &fileInfo{}
if !atomic.CompareAndSwapPointer(
(*unsafe.Pointer)(unsafe.Pointer(pinfo)), // adrr
nil, // old
unsafe.Pointer(fi), // new
) {
fi = (*fileInfo)(atomic.LoadPointer(ptr))
}
}
return fi
}

// StatDirEntry returns a [fs.FileInfo] describing the named file ([os.Stat]).
// If de is a [fastwalk.DirEntry] its Stat method is used and the returned
// FileInfo may be cached from a prior call to Stat. If a cached result is not
// desired, users should just call [os.Stat] directly.
// StatDirEntry returns a [fs.FileInfo] describing the named file. If de is a
// [fastwalk.DirEntry] its Stat method is used. Otherwise, [os.Stat] is called
// on the path. Therefore, de should be the DirEntry describing path.
//
// This is a helper function for calling Stat on the DirEntry passed to the
// walkFn argument to [Walk].
// Note: This function was added when fastwalk used to always cache the result
// of DirEntry.Stat. Now that fastwalk no longer explicitly caches the result
// of Stat this function is slightly less useful and is equivalent to the below
// code:
//
// The path argument is only used if de is not of type [fastwalk.DirEntry].
// Therefore, de should be the DirEntry describing path.
// if d, ok := de.(DirEntry); ok {
// return d.Stat()
// }
// return os.Stat(path)
func StatDirEntry(path string, de fs.DirEntry) (fs.FileInfo, error) {
if de == nil {
return nil, &os.PathError{Op: "stat", Path: path, Err: syscall.EINVAL}
}
if de.Type()&os.ModeSymlink == 0 {
return de.Info()
}
if d, ok := de.(DirEntry); ok {
return d.Stat()
}
Expand Down
16 changes: 8 additions & 8 deletions dirent_portable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package fastwalk
import (
"io/fs"
"os"
"runtime"
"sort"
"sync"

Expand All @@ -19,7 +20,6 @@ var _ DirEntry = (*portableDirent)(nil)
type portableDirent struct {
fs.DirEntry
parent string
stat *fileInfo
depth uint32
}

Expand All @@ -32,14 +32,14 @@ func (d *portableDirent) Depth() int {
}

func (d *portableDirent) Stat() (fs.FileInfo, error) {
if d.DirEntry.Type()&os.ModeSymlink == 0 {
return d.DirEntry.Info()
if runtime.GOOS == "windows" {
// On Windows use Info() if the file is not a symlink since
// IIRC the result is cached when the FileInfo is created.
if d.DirEntry.Type()&fs.ModeSymlink == 0 {
return d.DirEntry.Info()
}
}
stat := loadFileInfo(&d.stat)
stat.once.Do(func() {
stat.FileInfo, stat.err = os.Stat(d.parent + string(os.PathSeparator) + d.Name())
})
return stat.FileInfo, stat.err
return os.Stat(d.parent + string(os.PathSeparator) + d.Name())
}

func newDirEntry(dirName string, info fs.DirEntry, depth int) DirEntry {
Expand Down
88 changes: 0 additions & 88 deletions dirent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"io/fs"
"os"
"path/filepath"
"runtime"
"sync"
"testing"

"github.com/charlievieth/fastwalk"
Expand Down Expand Up @@ -75,91 +73,5 @@ func TestDirent(t *testing.T) {
t.Errorf("lstat mismatch\n got:\n%s\nwant:\n%s",
fastwalk.FormatFileInfo(got), fastwalk.FormatFileInfo(want))
}
fi, err := fileEnt.Info()
if err != nil {
t.Fatal(err)
}
if fi != got {
t.Error("failed to return or cache FileInfo")
}
de := fileEnt.(fastwalk.DirEntry)
fi, err = de.Stat()
if err != nil {
t.Fatal(err)
}
if fi != got {
t.Error("failed to use cached Info result for non-symlink")
}
})

t.Run("Parallel", func(t *testing.T) {
testParallel := func(t *testing.T, de fs.DirEntry, fn func() (fs.FileInfo, error)) {
numCPU := runtime.NumCPU()

infos := make([][]fs.FileInfo, numCPU)
for i := range infos {
infos[i] = make([]fs.FileInfo, 100)
}

// Start all the goroutines at the same time to
// maximise the chance of a race
start := make(chan struct{})
var wg, ready sync.WaitGroup
ready.Add(numCPU)
wg.Add(numCPU)
for i := 0; i < numCPU; i++ {
go func(fis []fs.FileInfo, de fs.DirEntry) {
defer wg.Done()
ready.Done()
<-start
for i := range fis {
fis[i], _ = de.Info()
}
}(infos[i], de)
}

ready.Wait()
close(start) // start all goroutines at once
wg.Wait()

first := infos[0][0]
if first == nil {
t.Fatal("failed to stat file:", de.Name())
}
for _, fis := range infos {
for _, fi := range fis {
if fi != first {
t.Errorf("Expected the same fs.FileInfo to always "+
"be returned got: %#v want: %#v", fi, first)
}
}
}
}

t.Run("File", func(t *testing.T) {
t.Run("Stat", func(t *testing.T) {
_, fileEnt := getDirEnts(t)
de := fileEnt.(fastwalk.DirEntry)
testParallel(t, de, de.Stat)
})
t.Run("Lstat", func(t *testing.T) {
_, fileEnt := getDirEnts(t)
de := fileEnt.(fastwalk.DirEntry)
testParallel(t, de, de.Info)
})
})

t.Run("Link", func(t *testing.T) {
t.Run("Stat", func(t *testing.T) {
linkEnt, _ := getDirEnts(t)
de := linkEnt.(fastwalk.DirEntry)
testParallel(t, de, de.Stat)
})
t.Run("Lstat", func(t *testing.T) {
linkEnt, _ := getDirEnts(t)
de := linkEnt.(fastwalk.DirEntry)
testParallel(t, de, de.Info)
})
})
})
}
28 changes: 3 additions & 25 deletions dirent_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ type unixDirent struct {
name string
typ fs.FileMode
depth uint32 // uint32 so that we can pack it next to typ
info *fileInfo
stat *fileInfo
}

func (d *unixDirent) Name() string { return d.name }
Expand All @@ -27,22 +25,11 @@ func (d *unixDirent) Depth() int { return int(d.depth) }
func (d *unixDirent) String() string { return fmtdirent.FormatDirEntry(d) }

func (d *unixDirent) Info() (fs.FileInfo, error) {
info := loadFileInfo(&d.info)
info.once.Do(func() {
info.FileInfo, info.err = os.Lstat(d.parent + "/" + d.name)
})
return info.FileInfo, info.err
return os.Lstat(d.parent + "/" + d.name)
}

func (d *unixDirent) Stat() (fs.FileInfo, error) {
if d.typ&os.ModeSymlink == 0 {
return d.Info()
}
stat := loadFileInfo(&d.stat)
stat.once.Do(func() {
stat.FileInfo, stat.err = os.Stat(d.parent + "/" + d.name)
})
return stat.FileInfo, stat.err
return os.Stat(d.parent + "/" + d.name)
}

func newUnixDirent(parent, name string, typ fs.FileMode, depth int) *unixDirent {
Expand All @@ -55,16 +42,7 @@ func newUnixDirent(parent, name string, typ fs.FileMode, depth int) *unixDirent
}

func fileInfoToDirEntry(dirname string, fi fs.FileInfo) DirEntry {
info := &fileInfo{
FileInfo: fi,
}
info.once.Do(func() {})
return &unixDirent{
parent: dirname,
name: fi.Name(),
typ: fi.Mode().Type(),
info: info,
}
return newUnixDirent(dirname, fi.Name(), fi.Mode().Type(), 0)
}

var direntSlicePool = sync.Pool{
Expand Down
84 changes: 20 additions & 64 deletions dirent_unix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,8 @@ import (
"reflect"
"runtime"
"sync"
"sync/atomic"
"testing"
"time"
"unsafe"
)

func testUnixDirentParallel(t *testing.T, ent *unixDirent, want fs.FileInfo,
Expand All @@ -23,7 +21,7 @@ func testUnixDirentParallel(t *testing.T, ent *unixDirent, want fs.FileInfo,
return fi1.Name() == fi2.Name() &&
fi1.Size() == fi2.Size() &&
fi1.Mode() == fi2.Mode() &&
fi1.ModTime() == fi2.ModTime() &&
fi1.ModTime().Equal(fi2.ModTime()) &&
fi1.IsDir() == fi2.IsDir() &&
os.SameFile(fi1, fi2)
}
Expand All @@ -38,10 +36,6 @@ func testUnixDirentParallel(t *testing.T, ent *unixDirent, want fs.FileInfo,

var wg sync.WaitGroup
start := make(chan struct{})
var mu sync.Mutex
infos := make(map[*fileInfo]int)
stats := make(map[*fileInfo]int)

for i := 0; i < numCPU; i++ {
wg.Add(1)
go func() {
Expand All @@ -53,12 +47,6 @@ func testUnixDirentParallel(t *testing.T, ent *unixDirent, want fs.FileInfo,
t.Error(err)
return
}
info := (*fileInfo)(atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&ent.info))))
stat := (*fileInfo)(atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&ent.stat))))
mu.Lock()
infos[info]++
stats[stat]++
mu.Unlock()
if !sameFile(fi, want) {
t.Errorf("FileInfo not equal:\nwant: %s\ngot: %s\n",
FormatFileInfo(want), FormatFileInfo(fi))
Expand All @@ -70,8 +58,6 @@ func testUnixDirentParallel(t *testing.T, ent *unixDirent, want fs.FileInfo,

close(start)
wg.Wait()

t.Logf("Infos: %d Stats: %d\n", len(infos), len(stats))
}

func TestUnixDirent(t *testing.T) {
Expand Down Expand Up @@ -222,24 +208,6 @@ func TestSortDirents(t *testing.T) {
})
}

func BenchmarkUnixDirentLoadFileInfo(b *testing.B) {
wd, err := os.Getwd()
if err != nil {
b.Fatal(err)
}
fi, err := os.Lstat(wd)
if err != nil {
b.Fatal(err)
}
parent, name := filepath.Split(wd)
d := newUnixDirent(parent, name, fi.Mode().Type(), 0)

for i := 0; i < b.N; i++ {
loadFileInfo(&d.info)
d.info = nil
}
}

func BenchmarkUnixDirentInfo(b *testing.B) {
wd, err := os.Getwd()
if err != nil {
Expand All @@ -250,38 +218,26 @@ func BenchmarkUnixDirentInfo(b *testing.B) {
b.Fatal(err)
}
parent, name := filepath.Split(wd)
d := newUnixDirent(parent, name, fi.Mode().Type(), 0)

for i := 0; i < b.N; i++ {
fi, err := d.Info()
if err != nil {
b.Fatal(err)
}
if fi == nil {
b.Fatal("Nil FileInfo")
b.Run("Serial", func(b *testing.B) {
d := newUnixDirent(parent, name, fi.Mode().Type(), 0)
for i := 0; i < b.N; i++ {
_, err := d.Info()
if err != nil {
b.Fatal(err)
}
}
}
}

func BenchmarkUnixDirentStat(b *testing.B) {
wd, err := os.Getwd()
if err != nil {
b.Fatal(err)
}
fi, err := os.Lstat(wd)
if err != nil {
b.Fatal(err)
}
parent, name := filepath.Split(wd)
d := newUnixDirent(parent, name, fi.Mode().Type(), 0)
})

for i := 0; i < b.N; i++ {
fi, err := d.Stat()
if err != nil {
b.Fatal(err)
}
if fi == nil {
b.Fatal("Nil FileInfo")
}
}
b.Run("Parallel", func(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
d := newUnixDirent(parent, name, fi.Mode().Type(), 0)
for pb.Next() {
_, err := d.Info()
if err != nil {
b.Fatal(err)
}
}
})
})
}
Loading
Loading