From f5211ee3fc6cf91e9b2862b85b9b33af581a3451 Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Sun, 7 May 2023 23:50:57 -0700 Subject: [PATCH] Change to Readdirnames for some cases There was a couple uses of Readdir/ReadDir here where the only thing the return value was used for was the Name of the entry. This is exactly what Readdirnames returns, so we can avoid the overhead of making/returning a bunch of interfaces and calling lstat everytime in the case of Readdir(-1). https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/os/dir_unix.go;l=114-137 Signed-off-by: Danny Canter --- content/local/store.go | 10 ++++------ pkg/cri/opts/container.go | 8 +++++++- runtime/v2/shim_load.go | 27 ++++++++++++++++++++++----- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/content/local/store.go b/content/local/store.go index baae3565b..10b2f0a04 100644 --- a/content/local/store.go +++ b/content/local/store.go @@ -295,10 +295,9 @@ func (s *store) ListStatuses(ctx context.Context, fs ...string) ([]content.Statu if err != nil { return nil, err } - defer fp.Close() - fis, err := fp.Readdir(-1) + fis, err := fp.Readdirnames(-1) if err != nil { return nil, err } @@ -310,7 +309,7 @@ func (s *store) ListStatuses(ctx context.Context, fs ...string) ([]content.Statu var active []content.Status for _, fi := range fis { - p := filepath.Join(s.root, "ingest", fi.Name()) + p := filepath.Join(s.root, "ingest", fi) stat, err := s.status(p) if err != nil { if !os.IsNotExist(err) { @@ -345,16 +344,15 @@ func (s *store) WalkStatusRefs(ctx context.Context, fn func(string) error) error if err != nil { return err } - defer fp.Close() - fis, err := fp.Readdir(-1) + fis, err := fp.Readdirnames(-1) if err != nil { return err } for _, fi := range fis { - rf := filepath.Join(s.root, "ingest", fi.Name(), "ref") + rf := filepath.Join(s.root, "ingest", fi, "ref") ref, err := readFileString(rf) if err != nil { diff --git a/pkg/cri/opts/container.go b/pkg/cri/opts/container.go index 0f0c0d9b3..c7f226b13 100644 --- a/pkg/cri/opts/container.go +++ b/pkg/cri/opts/container.go @@ -121,7 +121,13 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { // copyExistingContents copies from the source to the destination and // ensures the ownership is appropriately set. func copyExistingContents(source, destination string) error { - dstList, err := os.ReadDir(destination) + f, err := os.Open(destination) + if err != nil { + return err + } + defer f.Close() + + dstList, err := f.Readdirnames(-1) if err != nil { return err } diff --git a/runtime/v2/shim_load.go b/runtime/v2/shim_load.go index 7214bfc6a..d8675b392 100644 --- a/runtime/v2/shim_load.go +++ b/runtime/v2/shim_load.go @@ -83,12 +83,21 @@ func (m *ShimManager) loadShims(ctx context.Context) error { return err } // fast path - bf, err := os.ReadDir(bundle.Path) + f, err := os.Open(bundle.Path) if err != nil { bundle.Delete() log.G(ctx).WithError(err).Errorf("fast path read bundle path for %s", bundle.Path) continue } + + bf, err := f.Readdirnames(-1) + if err != nil { + f.Close() + bundle.Delete() + log.G(ctx).WithError(err).Errorf("fast path read bundle path for %s", bundle.Path) + continue + } + f.Close() if len(bf) == 0 { bundle.Delete() continue @@ -177,16 +186,24 @@ func (m *ShimManager) cleanupWorkDirs(ctx context.Context) error { if err != nil { return err } - dirs, err := os.ReadDir(filepath.Join(m.root, ns)) + + f, err := os.Open(filepath.Join(m.root, ns)) if err != nil { return err } - for _, d := range dirs { + defer f.Close() + + dirs, err := f.Readdirnames(-1) + if err != nil { + return err + } + + for _, dir := range dirs { // if the task was not loaded, cleanup and empty working directory // this can happen on a reboot where /run for the bundle state is cleaned up // but that persistent working dir is left - if _, err := m.shims.Get(ctx, d.Name()); err != nil { - path := filepath.Join(m.root, ns, d.Name()) + if _, err := m.shims.Get(ctx, dir); err != nil { + path := filepath.Join(m.root, ns, dir) if err := os.RemoveAll(path); err != nil { log.G(ctx).WithError(err).Errorf("cleanup working dir %s", path) }