From 9c3c8e038a76426a8c00dfdab4d6c1a4d781443d Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 5 Apr 2022 09:35:27 +0200 Subject: [PATCH] Add cache for mounts This change adds a cache to the mounts type. This means that if called to get a list of folders, for example, the result is reused instead of recalculated. This also avoids duplicate logging. Signed-off-by: Evan Lezar --- internal/discover/csv.go | 4 ++-- internal/discover/csv_test.go | 10 +++++----- internal/discover/mounts.go | 15 ++++++++++++++- internal/discover/mounts_test.go | 15 ++++++++------- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/internal/discover/csv.go b/internal/discover/csv.go index 59a1eed4..2a6cf6c9 100644 --- a/internal/discover/csv.go +++ b/internal/discover/csv.go @@ -119,7 +119,7 @@ func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]loo // Mounts returns the discovered mounts for the csvDiscoverer. // Note that if the discoverer is for the device MountSpecType, the list of mounts is empty. -func (d csvDiscoverer) Mounts() ([]Mount, error) { +func (d *csvDiscoverer) Mounts() ([]Mount, error) { if d.mountType == csv.MountSpecDev { return d.None.Mounts() } @@ -129,7 +129,7 @@ func (d csvDiscoverer) Mounts() ([]Mount, error) { // Devices returns the discovered devices for the csvDiscoverer. // Note that if the discoverer is not for the device MountSpecType, the list of devices is empty. -func (d csvDiscoverer) Devices() ([]Device, error) { +func (d *csvDiscoverer) Devices() ([]Device, error) { if d.mountType != csv.MountSpecDev { return d.None.Devices() } diff --git a/internal/discover/csv_test.go b/internal/discover/csv_test.go index f11d7af0..ac0ef482 100644 --- a/internal/discover/csv_test.go +++ b/internal/discover/csv_test.go @@ -31,7 +31,7 @@ func TestCSVDiscoverer(t *testing.T) { testCases := []struct { description string - input csvDiscoverer + input *csvDiscoverer expectedMounts []Mount expectedMountsError error expectedDevicesError error @@ -39,7 +39,7 @@ func TestCSVDiscoverer(t *testing.T) { }{ { description: "dev mounts are empty", - input: csvDiscoverer{ + input: &csvDiscoverer{ mounts: mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(string) ([]string, error) { @@ -54,14 +54,14 @@ func TestCSVDiscoverer(t *testing.T) { }, { description: "dev devices returns error for nil lookup", - input: csvDiscoverer{ + input: &csvDiscoverer{ mountType: "dev", }, expectedDevicesError: fmt.Errorf("no lookup defined"), }, { description: "lib devices are empty", - input: csvDiscoverer{ + input: &csvDiscoverer{ mounts: mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(string) ([]string, error) { @@ -76,7 +76,7 @@ func TestCSVDiscoverer(t *testing.T) { }, { description: "lib mounts returns error for nil lookup", - input: csvDiscoverer{ + input: &csvDiscoverer{ mountType: "lib", }, expectedMountsError: fmt.Errorf("no lookup defined"), diff --git a/internal/discover/mounts.go b/internal/discover/mounts.go index f294e522..07650509 100644 --- a/internal/discover/mounts.go +++ b/internal/discover/mounts.go @@ -18,6 +18,7 @@ package discover import ( "fmt" + "sync" "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" "github.com/sirupsen/logrus" @@ -31,15 +32,25 @@ type mounts struct { logger *logrus.Logger lookup lookup.Locator required []string + sync.Mutex + cache []Mount } var _ Discover = (*mounts)(nil) -func (d mounts) Mounts() ([]Mount, error) { +func (d *mounts) Mounts() ([]Mount, error) { if d.lookup == nil { return nil, fmt.Errorf("no lookup defined") } + if d.cache != nil { + d.logger.Debugf("returning cached mounts") + return d.cache, nil + } + + d.Lock() + defer d.Unlock() + paths := make(map[string]bool) for _, candidate := range d.required { @@ -68,5 +79,7 @@ func (d mounts) Mounts() ([]Mount, error) { mounts = append(mounts, mount) } + d.cache = mounts + return mounts, nil } diff --git a/internal/discover/mounts_test.go b/internal/discover/mounts_test.go index 35e68e76..b443d18c 100644 --- a/internal/discover/mounts_test.go +++ b/internal/discover/mounts_test.go @@ -41,16 +41,17 @@ func TestMounts(t *testing.T) { description string expectedError error expectedMounts []Mount - input mounts + input *mounts }{ { description: "nill lookup returns error", expectedError: fmt.Errorf("no lookup defined"), + input: &mounts{}, }, { description: "empty required returns no mounts", expectedError: nil, - input: mounts{ + input: &mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(string) ([]string, error) { return []string{"located"}, nil @@ -61,7 +62,7 @@ func TestMounts(t *testing.T) { { description: "required returns located", expectedError: nil, - input: mounts{ + input: &mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(string) ([]string, error) { return []string{"located"}, nil @@ -74,7 +75,7 @@ func TestMounts(t *testing.T) { { description: "mounts removes located duplicates", expectedError: nil, - input: mounts{ + input: &mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(string) ([]string, error) { return []string{"located"}, nil @@ -86,7 +87,7 @@ func TestMounts(t *testing.T) { }, { description: "mounts skips located errors", - input: mounts{ + input: &mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(s string) ([]string, error) { if s == "error" { @@ -101,7 +102,7 @@ func TestMounts(t *testing.T) { }, { description: "mounts skips unlocated", - input: mounts{ + input: &mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(s string) ([]string, error) { if s == "empty" { @@ -116,7 +117,7 @@ func TestMounts(t *testing.T) { }, { description: "mounts skips unlocated", - input: mounts{ + input: &mounts{ lookup: &lookup.LocatorMock{ LocateFunc: func(s string) ([]string, error) { if s == "multiple" {