Skip to content

Commit

Permalink
wasi: benchmarks fd_readdir for both real and fake file systems (#910)
Browse files Browse the repository at this point in the history 
It is likely we'll have to special-case real files in wasi, as wasi-libc
makes behavioral choices based on presence of non-zero inode data. This
adds a defensive benchmark, so that we know what our performance base
case is. This adds both real and fake file systems as both patterns are
in use today.

Signed-off-by: Adrian Cole <[email protected]>
  • Loading branch information
@codefromthecrypt
codefromthecrypt authored Dec 11, 2022
1 parent a9f402d commit e49995c
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 70 deletions.
22 changes: 14 additions & 8 deletions imports/wasi_snapshot_preview1/fs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,8 @@ func fdReadFn(ctx context.Context, mod api.Module, stack []uint64) (nread uint32
}

func fdReadOrPread(ctx context.Context, mod api.Module, stack []uint64, isPread bool) (uint32, Errno) {
sysCtx := mod.(*wasm.CallContext).Sys
mem := mod.Memory()
sysCtx := mod.(*wasm.CallContext).Sys

fd := uint32(stack[0])
iovs := uint32(stack[1])
Expand Down Expand Up @@ -698,6 +698,9 @@ var fdReaddir = proxyResultParams(&wasm.HostFunc{
}, fdReaddirName)

func fdReaddirFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, Errno) {
mem := mod.Memory()
fsc := mod.(*wasm.CallContext).Sys.FS(ctx)

fd := uint32(stack[0])
buf := uint32(stack[1])
bufLen := uint32(stack[2])
Expand All @@ -706,15 +709,21 @@ func fdReaddirFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, E
// it in such a way that becomes negative.
cookie := int64(stack[3])

// The bufLen must be enough to write a dirent. Otherwise, the caller can't
// read what the next cookie is.
if bufLen < direntSize {
return 0, ErrnoInval
}

// Validate the FD is a directory
rd, dir, errno := openedDir(ctx, mod, fd)
rd, dir, errno := openedDir(ctx, fsc, fd)
if errno != ErrnoSuccess {
return 0, errno
}

// expect a cookie only if we are continuing a read.
if cookie == 0 && dir.CountRead > 0 {
return 0, ErrnoInval // invalid as a cookie is minimally one.
return 0, ErrnoInval // cookie is minimally one.
}

// First, determine the maximum directory entries that can be encoded as
Expand Down Expand Up @@ -750,8 +759,6 @@ func fdReaddirFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, E
}
}

mem := mod.Memory()

// Determine how many dirents we can write, excluding a potentially
// truncated entry.
bufused, direntCount, writeTruncatedEntry := maxDirents(entries, bufLen)
Expand Down Expand Up @@ -930,8 +937,7 @@ func writeDirent(buf []byte, dNext uint64, dNamlen uint32, dType bool) {
}

// openedDir returns the directory and ErrnoSuccess if the fd points to a readable directory.
func openedDir(ctx context.Context, mod api.Module, fd uint32) (fs.ReadDirFile, *internalsys.ReadDir, Errno) {
fsc := mod.(*wasm.CallContext).Sys.FS(ctx)
func openedDir(ctx context.Context, fsc *internalsys.FSContext, fd uint32) (fs.ReadDirFile, *internalsys.ReadDir, Errno) {
if f, ok := fsc.OpenedFile(ctx, fd); !ok {
return nil, nil, ErrnoBadf
} else if d, ok := f.File.(fs.ReadDirFile); !ok {
Expand Down Expand Up @@ -1103,8 +1109,8 @@ var fdWrite = proxyResultParams(&wasm.HostFunc{
}, fdWriteName)

func fdWriteFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, Errno) {
sysCtx := mod.(*wasm.CallContext).Sys
mem := mod.Memory()
sysCtx := mod.(*wasm.CallContext).Sys

fd := uint32(stack[0])
iovs := uint32(stack[1])
Expand Down
107 changes: 56 additions & 51 deletions imports/wasi_snapshot_preview1/fs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,7 @@ func Test_fdReaddir(t *testing.T) {
tests := []struct {
name string
dir func() *internalsys.FileEntry
buf, bufLen uint32
bufLen uint32
cookie int64
expectedMem []byte
expectedMemSize int
Expand All @@ -1263,7 +1263,7 @@ func Test_fdReaddir(t *testing.T) {

return &internalsys.FileEntry{File: dir}
},
buf: 0, bufLen: 1,
bufLen: direntSize,
cookie: 0,
expectedBufused: 0,
expectedMem: []byte{},
Expand All @@ -1277,7 +1277,7 @@ func Test_fdReaddir(t *testing.T) {

return &internalsys.FileEntry{File: dir}
},
buf: 0, bufLen: 4096,
bufLen: 4096,
cookie: 0,
expectedBufused: 78, // length of all entries
expectedMem: append(append(dirent1, dirent2...), dirent3...),
Expand All @@ -1286,23 +1286,6 @@ func Test_fdReaddir(t *testing.T) {
Entries: testDirEntries,
},
},
{
name: "can't read",
dir: func() *internalsys.FileEntry {
dir, err := fdReadDirFs.Open("dir")
require.NoError(t, err)

return &internalsys.FileEntry{File: dir}
},
buf: 0, bufLen: 23, // length is too short for header
cookie: 0,
expectedBufused: 23, // == bufLen which is the size of the dirent
expectedMem: nil,
expectedReadDir: &internalsys.ReadDir{
CountRead: 2,
Entries: testDirEntries[:2],
},
},
{
name: "can't read name",
dir: func() *internalsys.FileEntry {
Expand All @@ -1311,7 +1294,7 @@ func Test_fdReaddir(t *testing.T) {

return &internalsys.FileEntry{File: dir}
},
buf: 0, bufLen: 24, // length is long enough for first, but not the name.
bufLen: 24, // length is long enough for first, but not the name.
cookie: 0,
expectedBufused: 24, // == bufLen which is the size of the dirent
expectedMem: dirent1[:24], // header without name
Expand All @@ -1328,7 +1311,7 @@ func Test_fdReaddir(t *testing.T) {

return &internalsys.FileEntry{File: dir}
},
buf: 0, bufLen: 25, // length is long enough for first + the name, but not more.
bufLen: 25, // length is long enough for first + the name, but not more.
cookie: 0,
expectedBufused: 25, // length to read exactly first.
expectedMem: dirent1,
Expand All @@ -1353,7 +1336,7 @@ func Test_fdReaddir(t *testing.T) {
},
}
},
buf: 0, bufLen: 26, // length is long enough for exactly second.
bufLen: 26, // length is long enough for exactly second.
cookie: 1, // d_next of first
expectedBufused: 26, // length to read exactly second.
expectedMem: dirent2,
Expand All @@ -1378,7 +1361,7 @@ func Test_fdReaddir(t *testing.T) {
},
}
},
buf: 0, bufLen: 30, // length is longer than the second entry, but not long enough for a header.
bufLen: 30, // length is longer than the second entry, but not long enough for a header.
cookie: 1, // d_next of first
expectedBufused: 30, // length to read some more, but not enough for a header, so buf was exhausted.
expectedMem: dirent2,
Expand All @@ -1404,7 +1387,7 @@ func Test_fdReaddir(t *testing.T) {
},
}
},
buf: 0, bufLen: 50, // length is longer than the second entry + enough for the header of third.
bufLen: 50, // length is longer than the second entry + enough for the header of third.
cookie: 1, // d_next of first
expectedBufused: 50, // length to read exactly second and the header of third.
expectedMem: append(dirent2, dirent3[0:24]...),
Expand All @@ -1429,7 +1412,7 @@ func Test_fdReaddir(t *testing.T) {
},
}
},
buf: 0, bufLen: 53, // length is long enough for second and third.
bufLen: 53, // length is long enough for second and third.
cookie: 1, // d_next of first
expectedBufused: 53, // length to read exactly one second and third.
expectedMem: append(dirent2, dirent3...),
Expand All @@ -1454,7 +1437,7 @@ func Test_fdReaddir(t *testing.T) {
},
}
},
buf: 0, bufLen: 27, // length is long enough for exactly third.
bufLen: 27, // length is long enough for exactly third.
cookie: 2, // d_next of second.
expectedBufused: 27, // length to read exactly third.
expectedMem: dirent3,
Expand All @@ -1479,9 +1462,9 @@ func Test_fdReaddir(t *testing.T) {
},
}
},
buf: 0, bufLen: 100, // length is long enough for third and more, but there is nothing more.
cookie: 2, // d_next of second.
expectedBufused: 27, // length to read exactly third.
bufLen: 100, // length is long enough for third and more, but there is nothing more.
cookie: 2, // d_next of second.
expectedBufused: 27, // length to read exactly third.
expectedMem: dirent3,
expectedReadDir: &internalsys.ReadDir{
CountRead: 3,
Expand All @@ -1506,17 +1489,17 @@ func Test_fdReaddir(t *testing.T) {

maskMemory(t, testCtx, mod, int(tc.bufLen))

// use an arbitrarily high value for the buf used position.
resultBufused := uint32(16192)
resultBufused := uint32(0) // where to write the amount used out of bufLen
buf := uint32(8) // where to start the dirents
requireErrno(t, ErrnoSuccess, mod, fdReaddirName,
uint64(fd), uint64(tc.buf), uint64(tc.bufLen), uint64(tc.cookie), uint64(resultBufused))
uint64(fd), uint64(buf), uint64(tc.bufLen), uint64(tc.cookie), uint64(resultBufused))

// read back the bufused and compare memory against it
bufUsed, ok := mod.Memory().ReadUint32Le(testCtx, resultBufused)
require.True(t, ok)
require.Equal(t, tc.expectedBufused, bufUsed)

mem, ok := mod.Memory().Read(testCtx, tc.buf, bufUsed)
mem, ok := mod.Memory().Read(testCtx, buf, bufUsed)
require.True(t, ok)

if tc.expectedMem != nil {
Expand Down Expand Up @@ -1569,26 +1552,30 @@ func Test_fdReaddir_Errors(t *testing.T) {
`,
},
{
name: "invalid fd",
fd: 42, // arbitrary invalid fd
name: "invalid fd",
fd: 42, // arbitrary invalid fd
buf: 0, bufLen: direntSize, // enough to read the dirent
resultBufused: 1000, // arbitrary
expectedErrno: ErrnoBadf,
expectedLog: `
--> proxy.fd_readdir(fd=42,buf=0,buf_len=0,cookie=0,result.bufused=0)
--> wasi_snapshot_preview1.fd_readdir(fd=42,buf=0,buf_len=0,cookie=0,result.bufused=0)
==> wasi_snapshot_preview1.fdReaddir(fd=42,buf=0,buf_len=0,cookie=0)
--> proxy.fd_readdir(fd=42,buf=0,buf_len=24,cookie=0,result.bufused=1000)
--> wasi_snapshot_preview1.fd_readdir(fd=42,buf=0,buf_len=24,cookie=0,result.bufused=1000)
==> wasi_snapshot_preview1.fdReaddir(fd=42,buf=0,buf_len=24,cookie=0)
<== (bufused=0,EBADF)
<-- EBADF
<-- 8
`,
},
{
name: "not a dir",
fd: fileFD,
name: "not a dir",
fd: fileFD,
buf: 0, bufLen: direntSize, // enough to read the dirent
resultBufused: 1000, // arbitrary
expectedErrno: ErrnoNotdir,
expectedLog: `
--> proxy.fd_readdir(fd=5,buf=0,buf_len=0,cookie=0,result.bufused=0)
--> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=0,cookie=0,result.bufused=0)
==> wasi_snapshot_preview1.fdReaddir(fd=5,buf=0,buf_len=0,cookie=0)
--> proxy.fd_readdir(fd=5,buf=0,buf_len=24,cookie=0,result.bufused=1000)
--> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=24,cookie=0,result.bufused=1000)
==> wasi_snapshot_preview1.fdReaddir(fd=5,buf=0,buf_len=24,cookie=0)
<== (bufused=0,ENOTDIR)
<-- ENOTDIR
<-- 54
Expand Down Expand Up @@ -1625,16 +1612,34 @@ func Test_fdReaddir_Errors(t *testing.T) {
`,
},
{
name: "resultBufused is outside memory",
name: "bufLen must be enough to write a struct",
fd: dirFD,
buf: 0, bufLen: 1,
resultBufused: memLen,
expectedErrno: ErrnoFault,
resultBufused: 1000,
expectedErrno: ErrnoInval,
expectedLog: `
--> proxy.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=65536)
--> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=65536)
<-- EFAULT
<-- 21
--> proxy.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=1000)
--> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=1000)
==> wasi_snapshot_preview1.fdReaddir(fd=4,buf=0,buf_len=1,cookie=0)
<== (bufused=0,EINVAL)
<-- EINVAL
<-- 28
`,
},
{
name: "cookie invalid when no prior state",
fd: dirFD,
buf: 0, bufLen: 1000,
cookie: 1,
resultBufused: 2000,
expectedErrno: ErrnoInval,
expectedLog: `
--> proxy.fd_readdir(fd=4,buf=0,buf_len=1000,cookie=1,result.bufused=2000)
--> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=1000,cookie=1,result.bufused=2000)
==> wasi_snapshot_preview1.fdReaddir(fd=4,buf=0,buf_len=1000,cookie=1)
<== (bufused=0,EINVAL)
<-- EINVAL
<-- 28
`,
},
{
Expand Down
Loading

0 comments on commit e49995c

Please sign in to comment.