From 1b10f52d598e41f9ffe3a851da2da746f3d3a47a Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 23 Apr 2019 17:33:50 -0700 Subject: [PATCH] Remember file position during Readdir() The caller must call Readdir() at least twice to detect EOF. The old code was always restarting the directory search and then skipping elements already seen, effectively doubling the cost to read a directory. The code now remembers the last offset and doesn't reposition the cursor if next request comes at the same offset. PiperOrigin-RevId: 244957816 Change-Id: If21a8dc68b76614adbcf4301439adfda40f2643f --- runsc/fsgofer/fsgofer.go | 56 +++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index 60dad642f..c964a2a3b 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -215,6 +215,12 @@ type localFile struct { // readDirMu protects against concurrent Readdir calls. readDirMu sync.Mutex + + // lastDirentOffset is the last offset returned by Readdir(). If another call + // to Readdir is made at the same offset, the file doesn't need to be + // repositioned. This is an important optimization because the caller must + // always make one extra call to detect EOF (empty result, no error). + lastDirentOffset uint64 } func openAnyFileFromParent(parent *localFile, name string) (*fd.FD, string, error) { @@ -470,25 +476,14 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID) func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) { // Duplicate current file if 'names' is empty. if len(names) == 0 { - var newFile *fd.FD - if l.isOpen() { - // File mode may have changed when it was opened, so open a new one. - var err error - newFile, err = openAnyFile(l.hostPath, func(mode int) (*fd.FD, error) { - return fd.Open(l.hostPath, openFlags|mode, 0) - }) - if err != nil { - return nil, nil, extractErrno(err) - } - } else { - newFd, err := syscall.Dup(l.file.FD()) - if err != nil { - return nil, nil, extractErrno(err) - } - newFile = fd.New(newFd) + newFile, err := openAnyFile(l.hostPath, func(mode int) (*fd.FD, error) { + return fd.Open(l.hostPath, openFlags|mode, 0) + }) + if err != nil { + return nil, nil, extractErrno(err) } - stat, err := stat(int(newFile.FD())) + stat, err := stat(newFile.FD()) if err != nil { newFile.Close() return nil, nil, extractErrno(err) @@ -884,14 +879,30 @@ func (l *localFile) Readdir(offset uint64, count uint32) ([]p9.Dirent, error) { l.readDirMu.Lock() defer l.readDirMu.Unlock() - if _, err := syscall.Seek(l.file.FD(), 0, 0); err != nil { - return nil, extractErrno(err) + skip := uint64(0) + + // Check if the file is at the correct position already. If not, seek to the + // beginning and read the entire directory again. + if l.lastDirentOffset != offset { + if _, err := syscall.Seek(l.file.FD(), 0, 0); err != nil { + return nil, extractErrno(err) + } + skip = offset } - return l.readDirent(l.file.FD(), offset, count) + dirents, err := l.readDirent(l.file.FD(), offset, count, skip) + if err == nil { + // On success, remember the offset that was returned at the current + // position. + l.lastDirentOffset = offset + uint64(len(dirents)) + } else { + // On failure, the state is unknown, force call to seek() next time. + l.lastDirentOffset = math.MaxUint64 + } + return dirents, err } -func (l *localFile) readDirent(f int, offset uint64, count uint32) ([]p9.Dirent, error) { +func (l *localFile) readDirent(f int, offset uint64, count uint32, skip uint64) ([]p9.Dirent, error) { // Limit 'count' to cap the slice size that is returned. const maxCount = 100000 if count > maxCount { @@ -904,7 +915,6 @@ func (l *localFile) readDirent(f int, offset uint64, count uint32) ([]p9.Dirent, direntsBuf := make([]byte, 8192) names := make([]string, 0, 100) - skip := offset // Tracks the number of entries to skip. end := offset + uint64(count) for offset < end { dirSize, err := syscall.ReadDirent(f, direntsBuf) @@ -912,7 +922,7 @@ func (l *localFile) readDirent(f int, offset uint64, count uint32) ([]p9.Dirent, return dirents, err } if dirSize <= 0 { - return dirents, nil // EOF + return dirents, nil } names := names[:0]