wasi: shares more cached stat info to reduce impact of inode requirement (#1161)

We formerly cached only the directory type, to avoid re-stat'ing the
same directory many times. Since we are there, we can also cache the
inode, which is strictly required by wasi and costly to fetch. Note:
this only affects the directory: its contents still need a potential
fan-out of stats which will be handled in another change.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
This commit is contained in:
Crypt Keeper
2023-02-25 07:28:45 +08:00
committed by GitHub
parent 118631be9e
commit 1e7660fc4f
8 changed files with 198 additions and 87 deletions

View File

@@ -489,6 +489,7 @@ func fdFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) Er
}
}
// TODO: this should work against the file descriptor not its last name!
if err := f.FS.Utimes(f.Name, atime, mtime); err != nil {
return ToErrno(err)
}
@@ -841,7 +842,7 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) Errno {
var err error
if f, ok := fsc.LookupFile(fd); !ok {
return ErrnoBadf
} else if dirents, err = dotDirents(f.File); err != nil {
} else if dirents, err = dotDirents(f); err != nil {
return ToErrno(err)
}
dir.Dirents = dirents
@@ -851,23 +852,13 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) Errno {
// Check if we have maxDirEntries, and read more from the FS as needed.
if entryCount := len(dirents); entryCount < maxDirEntries {
l, err := platform.Readdir(rd, maxDirEntries-entryCount)
if err == io.EOF { // EOF is not an error
} else if err != nil {
if errno = ToErrno(err); errno == ErrnoNoent {
// Only on Linux platforms, ReadDir returns ErrNotExist for the "removed while opened"
// directories. In order to provide consistent behavior, ignore it.
// See https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs.zig#L635-L637
//
// TODO: Once we have our own File type, we should punt this into the behind ReadDir method above.
} else {
return errno
}
} else {
dir.CountRead += uint64(len(l))
dirents = append(dirents, l...)
// Replace the cache with up to maxDirEntries, starting at cookie.
dir.Dirents = dirents
if errno = ToErrno(err); errno != ErrnoSuccess {
return errno
}
dir.CountRead += uint64(len(l))
dirents = append(dirents, l...)
// Replace the cache with up to maxDirEntries, starting at cookie.
dir.Dirents = dirents
}
// Determine how many dirents we can write, excluding a potentially
@@ -899,13 +890,15 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) Errno {
// dotDirents returns "." and "..", where "." has a real stat because
// wasi-testsuite does inode validation.
func dotDirents(f fs.File) ([]*platform.Dirent, error) {
var st platform.Stat_t
if err := platform.StatFile(f, &st); err != nil {
func dotDirents(f *sys.FileEntry) ([]*platform.Dirent, error) {
ino, ft, err := f.CachedStat()
if err != nil {
return nil, err
} else if ft.Type() != fs.ModeDir {
return nil, syscall.ENOTDIR
}
return []*platform.Dirent{
{Name: ".", Ino: st.Ino, Type: fs.ModeDir},
{Name: ".", Ino: ino, Type: fs.ModeDir},
{Name: "..", Type: fs.ModeDir},
}, nil
}
@@ -1062,7 +1055,9 @@ func writeDirent(buf []byte, dNext uint64, dNamlen uint32, dType bool) {
func openedDir(fsc *sys.FSContext, fd uint32) (fs.File, *sys.ReadDir, Errno) {
if f, ok := fsc.LookupFile(fd); !ok {
return nil, nil, ErrnoBadf
} else if !f.IsDir() {
} else if _, ft, err := f.CachedStat(); err != nil {
return nil, nil, ToErrno(err)
} else if ft.Type() != fs.ModeDir {
// fd_readdir docs don't indicate whether to return ErrnoNotdir or
// ErrnoBadf. It has been noticed that rust will crash on ErrnoNotdir,
// and POSIX C ref seems to not return this, so we don't either.
@@ -1151,7 +1146,11 @@ func fdSeekFn(_ context.Context, mod api.Module, params []uint64) Errno {
if f, ok := fsc.LookupFile(fd); !ok {
return ErrnoBadf
// fs.FS doesn't declare io.Seeker, but implementations such as os.File implement it.
} else if seeker, ok = f.File.(io.Seeker); !ok || f.IsDir() {
} else if _, ft, err := f.CachedStat(); err != nil {
return ToErrno(err)
} else if ft.Type() == fs.ModeDir {
return ErrnoBadf
} else if seeker, ok = f.File.(io.Seeker); !ok {
return ErrnoBadf
}
@@ -1600,7 +1599,10 @@ func pathOpenFn(_ context.Context, mod api.Module, params []uint64) Errno {
if isDir {
if f, ok := fsc.LookupFile(newFD); !ok {
return ErrnoBadf // unexpected
} else if !f.IsDir() {
} else if _, ft, err := f.CachedStat(); err != nil {
_ = fsc.CloseFile(newFD)
return ToErrno(err)
} else if ft.Type() != fs.ModeDir {
_ = fsc.CloseFile(newFD)
return ErrnoNotdir
}
@@ -1638,7 +1640,9 @@ func atPath(fsc *sys.FSContext, mem api.Memory, dirFD, path, pathLen uint32) (sy
if f, ok := fsc.LookupFile(dirFD); !ok {
return nil, "", ErrnoBadf // closed
} else if !f.IsDir() {
} else if _, ft, err := f.CachedStat(); err != nil {
return nil, "", ToErrno(err)
} else if ft.Type() != fs.ModeDir {
return nil, "", ErrnoNotdir
} else if f.IsPreopen { // don't append the pre-open name
return f.FS, pathName, ErrnoSuccess
@@ -1862,7 +1866,9 @@ func pathSymlinkFn(_ context.Context, mod api.Module, params []uint64) Errno {
dir, ok := fsc.LookupFile(dirFD)
if !ok {
return ErrnoBadf // closed
} else if !dir.IsDir() {
} else if _, ft, err := dir.CachedStat(); err != nil {
return ToErrno(err)
} else if ft.Type() != fs.ModeDir {
return ErrnoNotdir
}

View File

@@ -2140,7 +2140,7 @@ func Test_fdReaddir(t *testing.T) {
}
func Test_fdReaddir_Rewind(t *testing.T) {
mod, r, _ := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.FS))
mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.FS))
defer r.Close(testCtx)
fsc := mod.(*wasm.CallContext).Sys.FS()
@@ -2191,6 +2191,14 @@ func Test_fdReaddir_Rewind(t *testing.T) {
resultBuf, ok = mem.Read(buf, usedAfterRewind)
require.True(t, ok)
require.Equal(t, dirents, resultBuf)
require.Equal(t, `
==> wasi_snapshot_preview1.fd_readdir(fd=4,buf=8,buf_len=200,cookie=0)
<== (bufused=129,errno=ESUCCESS)
==> wasi_snapshot_preview1.fd_readdir(fd=4,buf=8,buf_len=200,cookie=5)
<== (bufused=0,errno=ESUCCESS)
==> wasi_snapshot_preview1.fd_readdir(fd=4,buf=8,buf_len=200,cookie=0)
<== (bufused=129,errno=ESUCCESS)
`, "\n"+log.String())
}
func Test_fdReaddir_Errors(t *testing.T) {
@@ -2223,8 +2231,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
bufLen: 1000,
expectedErrno: ErrnoFault,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=65536,buf_len=1000,cookie=0,result.bufused=0)
<== errno=EFAULT
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=65536,buf_len=1000,cookie=0)
<== (bufused=,errno=EFAULT)
`,
},
{
@@ -2234,8 +2242,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
resultBufused: 1000, // arbitrary
expectedErrno: ErrnoBadf,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=42,buf=0,buf_len=24,cookie=0,result.bufused=1000)
<== errno=EBADF
==> wasi_snapshot_preview1.fd_readdir(fd=42,buf=0,buf_len=24,cookie=0)
<== (bufused=,errno=EBADF)
`,
},
{
@@ -2245,8 +2253,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
resultBufused: 1000, // arbitrary
expectedErrno: ErrnoBadf,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=24,cookie=0,result.bufused=1000)
<== errno=EBADF
==> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=24,cookie=0)
<== (bufused=,errno=EBADF)
`,
},
{
@@ -2256,8 +2264,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
bufLen: 1000,
expectedErrno: ErrnoFault,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=65535,buf_len=1000,cookie=0,result.bufused=0)
<== errno=EFAULT
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=65535,buf_len=1000,cookie=0)
<== (bufused=,errno=EFAULT)
`,
},
{
@@ -2267,8 +2275,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
resultBufused: 1000,
expectedErrno: ErrnoInval,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=1,cookie=0,result.bufused=1000)
<== errno=EINVAL
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=1,cookie=0)
<== (bufused=,errno=EINVAL)
`,
},
{
@@ -2279,8 +2287,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
resultBufused: 2000,
expectedErrno: ErrnoInval,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=1000,cookie=1,result.bufused=2000)
<== errno=EINVAL
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=1000,cookie=1)
<== (bufused=,errno=EINVAL)
`,
},
{
@@ -2292,8 +2300,8 @@ func Test_fdReaddir_Errors(t *testing.T) {
resultBufused: 2000,
expectedErrno: ErrnoInval,
expectedLog: `
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=1000,cookie=-1,result.bufused=2000)
<== errno=EINVAL
==> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=1000,cookie=-1)
<== (bufused=,errno=EINVAL)
`,
},
}
@@ -3451,7 +3459,9 @@ func Test_pathOpen(t *testing.T) {
expected: func(t *testing.T, fsc *sys.FSContext) {
f, ok := fsc.LookupFile(expectedOpenedFd)
require.True(t, ok)
require.True(t, f.IsDir())
_, ft, err := f.CachedStat()
require.NoError(t, err)
require.Equal(t, fs.ModeDir, ft)
},
expectedLog: `
==> wasi_snapshot_preview1.path_open(fd=3,dirflags=,path=dir,oflags=DIRECTORY,fs_rights_base=,fs_rights_inheriting=,fdflags=)
@@ -3466,7 +3476,9 @@ func Test_pathOpen(t *testing.T) {
expected: func(t *testing.T, fsc *sys.FSContext) {
f, ok := fsc.LookupFile(expectedOpenedFd)
require.True(t, ok)
require.True(t, f.IsDir())
_, ft, err := f.CachedStat()
require.NoError(t, err)
require.Equal(t, fs.ModeDir, ft)
},
expectedLog: `
==> wasi_snapshot_preview1.path_open(fd=3,dirflags=,path=dir,oflags=DIRECTORY,fs_rights_base=,fs_rights_inheriting=,fdflags=)

View File

@@ -67,7 +67,8 @@ func (d *Dirent) IsDir() bool {
// If n > 0, Readdir returns at most n entries or an error.
// If n <= 0, Readdir returns all remaining entries or an error.
//
// Note: The error will be nil or a syscall.Errno. No error is returned on EOF.
// Note: The error will be nil or a syscall.Errno. There's no error when the
// file is closed or removed while open. No error is returned on io.EOF.
func Readdir(f fs.File, n int) (dirents []*Dirent, err error) {
// ^^ case format is to match POSIX and similar to os.File.Readdir
@@ -76,10 +77,17 @@ func Readdir(f fs.File, n int) (dirents []*Dirent, err error) {
var entries []fs.DirEntry
entries, err = f.ReadDir(n)
if err == io.EOF {
err = nil
}
if err != nil {
break
err = nil // not an error
} else if err = UnwrapOSError(err); err != nil {
// Ignore errors when the file was closed or removed.
// See https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs.zig#L635-L637
switch err {
case syscall.EIO, syscall.EBADF: // closed while open
err = nil
case syscall.ENOENT: // Linux error when removed while open
err = nil
}
return
}
dirents = make([]*Dirent, 0, len(entries))
for _, e := range entries {
@@ -89,6 +97,5 @@ func Readdir(f fs.File, n int) (dirents []*Dirent, err error) {
default:
err = syscall.ENOTDIR
}
err = UnwrapOSError(err)
return
}

View File

@@ -3,6 +3,7 @@ package platform_test
import (
"io/fs"
"os"
"path"
"runtime"
"sort"
"syscall"
@@ -124,14 +125,12 @@ func TestReaddir(t *testing.T) {
require.Zero(t, len(dirents))
})
// windows and fstest.MapFS allow you to read a closed dir
if runtime.GOOS != "windows" && tc.name != "fstest.MapFS" {
t.Run("closed dir", func(t *testing.T) {
require.NoError(t, dirF.Close())
_, err := platform.Readdir(dirF, -1)
require.EqualErrno(t, syscall.EIO, err)
})
}
// Don't err if something else closed the directory while reading.
t.Run("closed dir", func(t *testing.T) {
require.NoError(t, dirF.Close())
_, err := platform.Readdir(dirF, -1)
require.NoError(t, err)
})
fileF, err := tc.fs.Open("empty.txt")
require.NoError(t, err)
@@ -157,4 +156,28 @@ func TestReaddir(t *testing.T) {
})
})
}
// Don't err if something else removed the directory while reading.
t.Run("removed while open", func(t *testing.T) {
dirF, err := dirFS.Open("dir")
require.NoError(t, err)
defer dirF.Close()
dirents, err := platform.Readdir(dirF, 1)
require.NoError(t, err)
require.Equal(t, 1, len(dirents))
// Speculatively try to remove even if it won't likely work
// on windows.
err = os.RemoveAll(path.Join(tmpDir, "dir"))
if err != nil && runtime.GOOS == "windows" {
t.Skip()
} else {
require.NoError(t, err)
}
_, err = platform.Readdir(dirF, 1)
require.NoError(t, err)
// don't validate the contents as due to caching it might be present.
})
}

View File

@@ -163,7 +163,8 @@ type FileEntry struct {
// FS is the filesystem associated with the pre-open.
FS sysfs.FS
isDirectory bool
// cachedStat includes fields that won't change while a file is open.
cachedStat *cachedStat
// File is always non-nil.
File fs.File
@@ -177,21 +178,40 @@ type FileEntry struct {
openPerm fs.FileMode
}
// IsDir returns true if the file is a directory.
func (f *FileEntry) IsDir() bool {
if f.isDirectory {
return true
type cachedStat struct {
// Ino is the file serial number, or zero if not available.
Ino uint64
// Type is the same as what's documented on platform.Dirent.
Type fs.FileMode
}
// CachedStat returns the cacheable parts of platform.Stat_t or an error if
// they couldn't be retrieved.
func (f *FileEntry) CachedStat() (ino uint64, fileType fs.FileMode, err error) {
if f.cachedStat == nil {
var st platform.Stat_t
if err = f.Stat(&st); err != nil {
return
}
f.cachedStat = &cachedStat{Ino: st.Ino, Type: st.Mode & fs.ModeType}
}
var stat platform.Stat_t
_ = f.Stat(&stat) // Maybe the file hasn't had stat yet.
return f.isDirectory
return f.cachedStat.Ino, f.cachedStat.Type, nil
}
// Stat returns the underlying stat of this file.
func (f *FileEntry) Stat(st *platform.Stat_t) (err error) {
err = platform.StatFile(f.File, st)
if ld, ok := f.File.(*lazyDir); ok {
var sf fs.File
if sf, err = ld.file(); err == nil {
err = platform.StatFile(sf, st)
}
} else {
err = platform.StatFile(f.File, st)
}
if err == nil {
f.isDirectory = st.Mode.IsDir()
f.cachedStat = &cachedStat{Ino: st.Ino, Type: st.Mode}
}
return
}
@@ -245,20 +265,18 @@ func NewFSContext(stdin io.Reader, stdout, stderr io.Writer, rootFS sysfs.FS) (f
preopens := comp.FS()
for i, p := range comp.GuestPaths() {
fsc.openedFiles.Insert(&FileEntry{
FS: preopens[i],
Name: p,
IsPreopen: true,
isDirectory: true,
File: &lazyDir{fs: rootFS},
FS: preopens[i],
Name: p,
IsPreopen: true,
File: &lazyDir{fs: rootFS},
})
}
} else {
fsc.openedFiles.Insert(&FileEntry{
FS: rootFS,
Name: "/",
IsPreopen: true,
isDirectory: true,
File: &lazyDir{fs: rootFS},
FS: rootFS,
Name: "/",
IsPreopen: true,
File: &lazyDir{fs: rootFS},
})
}
@@ -330,7 +348,9 @@ func (c *FSContext) ReOpenDir(fd uint32) (*FileEntry, error) {
f, ok := c.openedFiles.Lookup(fd)
if !ok {
return nil, syscall.EBADF
} else if !f.IsDir() {
} else if _, ft, err := f.CachedStat(); err != nil {
return nil, err
} else if ft.Type() != fs.ModeDir {
return nil, syscall.EISDIR
}
@@ -364,7 +384,9 @@ func (c *FSContext) ChangeOpenFlag(fd uint32, flag int) error {
f, ok := c.LookupFile(fd)
if !ok {
return syscall.EBADF
} else if f.IsDir() {
} else if _, ft, err := f.CachedStat(); err != nil {
return err
} else if ft.Type() == fs.ModeDir {
return syscall.EISDIR
}

View File

@@ -2,12 +2,13 @@ package sys
import (
"bytes"
"io/fs"
"testing"
"time"
"github.com/tetratelabs/wazero/internal/fstest"
"github.com/tetratelabs/wazero/internal/platform"
"github.com/tetratelabs/wazero/internal/sysfs"
testfs "github.com/tetratelabs/wazero/internal/testing/fs"
"github.com/tetratelabs/wazero/internal/testing/require"
"github.com/tetratelabs/wazero/sys"
)
@@ -28,7 +29,7 @@ func TestContext_WalltimeNanos(t *testing.T) {
}
func TestDefaultSysContext(t *testing.T) {
testFS := sysfs.Adapt(testfs.FS{})
testFS := sysfs.Adapt(fstest.FS)
sysCtx, err := NewContext(
0, // max
@@ -67,17 +68,54 @@ func TestDefaultSysContext(t *testing.T) {
expectedOpenedFiles.Insert(noopStdout)
expectedOpenedFiles.Insert(noopStderr)
expectedOpenedFiles.Insert(&FileEntry{
IsPreopen: true,
isDirectory: true,
Name: "/",
FS: testFS,
File: &lazyDir{fs: testFS},
IsPreopen: true,
Name: "/",
FS: testFS,
File: &lazyDir{fs: testFS},
})
require.Equal(t, expectedOpenedFiles, expectedFS.openedFiles)
require.Equal(t, expectedFS, sysCtx.FS())
}
func TestFileEntry_cachedStat(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
require.NoError(t, fstest.WriteTestFiles(tmpDir))
dirFS := sysfs.NewDirFS(tmpDir)
// get the expected inode
var st platform.Stat_t
require.NoError(t, platform.Stat(tmpDir, &st))
tests := []struct {
name string
fs sysfs.FS
expectedIno uint64
}{
{name: "sysfs.FS", fs: dirFS, expectedIno: st.Ino},
{name: "fs.FS", fs: sysfs.Adapt(fstest.FS), expectedIno: 0},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
fsc, _ := NewFSContext(nil, nil, nil, tc.fs)
defer fsc.Close(testCtx)
f, ok := fsc.LookupFile(FdPreopen)
require.True(t, ok)
ino, ft, err := f.CachedStat()
require.NoError(t, err)
require.Equal(t, fs.ModeDir, ft)
require.Equal(t, tc.expectedIno, ino)
require.Equal(t, &cachedStat{Ino: tc.expectedIno, Type: fs.ModeDir}, f.cachedStat)
})
}
}
func TestNewContext_Args(t *testing.T) {
tests := []struct {
name string

View File

@@ -266,6 +266,9 @@ var errnoToString = [...]string{
// error codes. For example, wasi-filesystem and GOOS=js don't map to these
// Errno.
func ToErrno(err error) Errno {
if err == nil {
return ErrnoSuccess
}
errno := platform.UnwrapOSError(err)
switch errno {

View File

@@ -145,7 +145,7 @@ func Config(fnd api.FunctionDefinition) (pSampler logging.ParamSampler, pLoggers
logger = logFsRightsBase(idx).Log
case "fs_rights_inheriting":
logger = logFsRightsInheriting(idx).Log
case "result.nread", "result.nwritten", "result.opened_fd", "result.nevents":
case "result.nread", "result.nwritten", "result.opened_fd", "result.nevents", "result.bufused":
name = resultParamName(name)
logger = logMemI32(idx).Log
rLoggers = append(rLoggers, resultParamLogger(name, logger))