lib/fs: Cache user lookups (#8496)

lib/fs/basicfs.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"os/user"
 	"path/filepath"
 	"strings"
 	"time"
@@ -46,8 +47,13 @@ type BasicFilesystem struct {
 	root            string
 	junctionsAsDirs bool
 	options         []Option
+	userCache       *userCache
+	groupCache      *groupCache
 }
 
+type userCache = valueCache[string, *user.User]
+type groupCache = valueCache[string, *user.Group]
+
 func newBasicFilesystem(root string, opts ...Option) *BasicFilesystem {
 	if root == "" {
 		root = "." // Otherwise "" becomes "/" below
@@ -84,8 +90,10 @@ func newBasicFilesystem(root string, opts ...Option) *BasicFilesystem {
 	}
 
 	fs := &BasicFilesystem{
-		root:    root,
-		options: opts,
+		root:       root,
+		options:    opts,
+		userCache:  newValueCache(time.Hour, user.LookupId),
+		groupCache: newValueCache(time.Hour, user.LookupGroupId),
 	}
 	for _, opt := range opts {
 		opt.apply(fs)

lib/fs/basicfs_platformdata_unix.go

@@ -14,5 +14,5 @@ import (
 )
 
 func (f *BasicFilesystem) PlatformData(name string) (protocol.PlatformData, error) {
-	return unixPlatformData(f, name)
+	return unixPlatformData(f, name, f.userCache, f.groupCache)
 }

lib/fs/basicfs_platformdata_windows.go

@@ -8,7 +8,6 @@ package fs
 
 import (
 	"fmt"
-	"os/user"
 
 	"github.com/syncthing/syncthing/lib/protocol"
 	"golang.org/x/sys/windows"
@@ -35,12 +34,10 @@ func (f *BasicFilesystem) PlatformData(name string) (protocol.PlatformData, erro
 		return protocol.PlatformData{}, fmt.Errorf("get owner for %s: %w", rootedName, err)
 	}
 
-	// The owner SID might represent a user or a group. We try to look it up
-	// as both, and set the appropriate fields in the OS data.
 	pd := &protocol.WindowsData{}
-	if us, err := user.LookupId(owner.String()); err == nil {
+	if us := f.userCache.lookup(owner.String()); us != nil {
 		pd.OwnerName = us.Username
-	} else if gr, err := user.LookupGroupId(owner.String()); err == nil {
+	} else if gr := f.groupCache.lookup(owner.String()); gr != nil {
 		pd.OwnerName = gr.Name
 		pd.OwnerIsGroup = true
 	} else {

lib/fs/basicfs_unix.go

@@ -16,7 +16,7 @@ import (
 	"strings"
 )
 
-func (BasicFilesystem) SymlinksSupported() bool {
+func (*BasicFilesystem) SymlinksSupported() bool {
 	return true
 }
 

lib/fs/fakefs.go

@@ -15,6 +15,7 @@ import (
 	"math/rand"
 	"net/url"
 	"os"
+	"os/user"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -61,6 +62,8 @@ type fakeFS struct {
 	insens      bool
 	withContent bool
 	latency     time.Duration
+	userCache   *userCache
+	groupCache  *groupCache
 }
 
 type fakeFSCounters struct {
@@ -109,6 +112,8 @@ func newFakeFilesystem(rootURI string, _ ...Option) *fakeFS {
 			mtime:     time.Now(),
 			children:  make(map[string]*fakeEntry),
 		},
+		userCache:  newValueCache(time.Hour, user.LookupId),
+		groupCache: newValueCache(time.Hour, user.LookupGroupId),
 	}
 
 	files, _ := strconv.Atoi(params.Get("files"))
@@ -658,7 +663,7 @@ func (fs *fakeFS) SameFile(fi1, fi2 FileInfo) bool {
 }
 
 func (fs *fakeFS) PlatformData(name string) (protocol.PlatformData, error) {
-	return unixPlatformData(fs, name)
+	return unixPlatformData(fs, name, fs.userCache, fs.groupCache)
 }
 
 func (*fakeFS) underlying() (Filesystem, bool) {

lib/fs/platform_common.go

@@ -7,8 +7,9 @@
 package fs
 
 import (
-	"os/user"
 	"strconv"
+	"sync"
+	"time"
 
 	"github.com/syncthing/syncthing/lib/protocol"
 )
@@ -16,7 +17,7 @@ import (
 // unixPlatformData is used on all platforms, because apart from being the
 // implementation for BasicFilesystem on Unixes it's also the implementation
 // in fakeFS.
-func unixPlatformData(fs Filesystem, name string) (protocol.PlatformData, error) {
+func unixPlatformData(fs Filesystem, name string, userCache *userCache, groupCache *groupCache) (protocol.PlatformData, error) {
 	stat, err := fs.Lstat(name)
 	if err != nil {
 		return protocol.PlatformData{}, err
@@ -24,8 +25,8 @@ func unixPlatformData(fs Filesystem, name string) (protocol.PlatformData, error)
 
 	ownerUID := stat.Owner()
 	ownerName := ""
-	if u, err := user.LookupId(strconv.Itoa(ownerUID)); err == nil && u.Username != "" {
-		ownerName = u.Username
+	if user := userCache.lookup(strconv.Itoa(ownerUID)); user != nil {
+		ownerName = user.Username
 	} else if ownerUID == 0 {
 		// We couldn't look up a name, but UID zero should be "root". This
 		// fixup works around the (unlikely) situation where the ownership
@@ -38,8 +39,8 @@ func unixPlatformData(fs Filesystem, name string) (protocol.PlatformData, error)
 
 	groupID := stat.Group()
 	groupName := ""
-	if g, err := user.LookupGroupId(strconv.Itoa(groupID)); err == nil && g.Name != "" {
-		groupName = g.Name
+	if group := groupCache.lookup(strconv.Itoa(ownerUID)); group != nil {
+		groupName = group.Name
 	} else if groupID == 0 {
 		groupName = "root"
 	}
@@ -53,3 +54,39 @@ func unixPlatformData(fs Filesystem, name string) (protocol.PlatformData, error)
 		},
 	}, nil
 }
+
+type valueCache[K comparable, V any] struct {
+	validity time.Duration
+	fill     func(K) (V, error)
+
+	mut   sync.Mutex
+	cache map[K]cacheEntry[V]
+}
+
+type cacheEntry[V any] struct {
+	value V
+	when  time.Time
+}
+
+func newValueCache[K comparable, V any](validity time.Duration, fill func(K) (V, error)) *valueCache[K, V] {
+	return &valueCache[K, V]{
+		validity: validity,
+		fill:     fill,
+		cache:    make(map[K]cacheEntry[V]),
+	}
+}
+
+func (c *valueCache[K, V]) lookup(key K) V {
+	c.mut.Lock()
+	defer c.mut.Unlock()
+	if e, ok := c.cache[key]; ok && time.Since(e.when) < c.validity {
+		return e.value
+	}
+	var e cacheEntry[V]
+	if val, err := c.fill(key); err == nil {
+		e.value = val
+	}
+	e.when = time.Now()
+	c.cache[key] = e
+	return e.value
+}

lib/scanner/walk_test.go

@@ -1028,3 +1028,25 @@ func testConfig() (Config, context.CancelFunc) {
 		EventLogger: evLogger,
 	}, cancel
 }
+
+func BenchmarkWalk(b *testing.B) {
+	testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, b.TempDir())
+
+	for i := 0; i < 100; i++ {
+		if err := testFs.Mkdir(fmt.Sprintf("dir%d", i), 0755); err != nil {
+			b.Fatal(err)
+		}
+		for j := 0; j < 100; j++ {
+			if fd, err := testFs.Create(fmt.Sprintf("dir%d/file%d", i, j)); err != nil {
+				b.Fatal(err)
+			} else {
+				fd.Close()
+			}
+		}
+	}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		walkDir(testFs, "/", nil, nil, 0)
+	}
+}