Browse Source

lib/fs: Reduce memory usage in xattrs handling (#9251)

This reduces allocations, in number and in size, while getting extended
attributes. This is mostly noticable when there is a large number of new
files to scan and we're running with the default scanProgressInterval --
then a queue of files is built in-memory, and this queue includes
extended attributes as part of file metadata. (Arguable it shouldn't,
but that's a more difficult and involved change.)

With 1M files to scan, each with one extended attribute, current peak
memory usage looks like this:

	Showing nodes accounting for 1425.30MB, 98.19% of 1451.64MB total
	Dropped 1435 nodes (cum <= 7.26MB)
	Showing top 10 nodes out of 54
	      flat  flat%   sum%        cum   cum%
976.56MB 67.27% 67.27% 976.56MB 67.27%
github.com/syncthing/syncthing/lib/fs.getXattr
305.44MB 21.04% 88.31% 305.44MB 21.04%
github.com/syncthing/syncthing/lib/scanner.(*walker).walk.func1
45.78MB 3.15% 91.47% 1045.23MB 72.00%
github.com/syncthing/syncthing/lib/fs.(*BasicFilesystem).GetXattr
22.89MB 1.58% 93.04% 22.89MB 1.58%
github.com/syncthing/syncthing/lib/fs.listXattr
22.89MB 1.58% 94.62% 22.89MB 1.58%
github.com/syncthing/syncthing/lib/protocol.(*PlatformData).SetXattrs
16MB 1.10% 95.72% 16.01MB 1.10%
github.com/syndtr/goleveldb/leveldb/memdb.New

After the change, it's this:

	Showing nodes accounting for 502.32MB, 95.70% of 524.88MB total
	Dropped 1400 nodes (cum <= 2.62MB)
	Showing top 10 nodes out of 91
	      flat  flat%   sum%        cum   cum%
305.43MB 58.19% 58.19% 305.43MB 58.19%
github.com/syncthing/syncthing/lib/scanner.(*walker).walk.func1
45.79MB 8.72% 66.91% 68.68MB 13.09%
github.com/syncthing/syncthing/lib/fs.(*BasicFilesystem).GetXattr
32MB 6.10% 73.01% 32.01MB 6.10%
github.com/syndtr/goleveldb/leveldb/memdb.New
22.89MB 4.36% 77.37% 22.89MB 4.36%
github.com/syncthing/syncthing/lib/fs.listXattr
22.89MB 4.36% 81.73% 22.89MB 4.36%
github.com/syncthing/syncthing/lib/protocol.(*PlatformData).SetXattrs
15.35MB 2.92% 84.66% 15.36MB 2.93%
github.com/syndtr/goleveldb/leveldb/util.(*BufferPool).Get
	   15.28MB  2.91% 87.57%    15.28MB  2.91%  strings.(*Builder).grow

(The usage for xattrs is reduced from 976 MB to 68 MB)
Jakob Borg 1 year ago
parent
commit
c1ec9a8826
1 changed files with 34 additions and 9 deletions
  1. 34 9
      lib/fs/basicfs_xattr_unix.go

+ 34 - 9
lib/fs/basicfs_xattr_unix.go

@@ -13,6 +13,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"sync"
 	"syscall"
 
 	"github.com/syncthing/syncthing/lib/protocol"
@@ -31,14 +32,13 @@ func (f *BasicFilesystem) GetXattr(path string, xattrFilter XattrFilter) ([]prot
 	}
 
 	res := make([]protocol.Xattr, 0, len(attrs))
-	var val, buf []byte
 	var totSize int
 	for _, attr := range attrs {
 		if !xattrFilter.Permit(attr) {
 			l.Debugf("get xattr %s: skipping attribute %q denied by filter", path, attr)
 			continue
 		}
-		val, buf, err = getXattr(path, attr, buf)
+		val, err := getXattr(path, attr)
 		var errNo syscall.Errno
 		if errors.As(err, &errNo) && errNo == 0x5d {
 			// ENOATTR, returned on BSD when asking for an attribute that
@@ -64,27 +64,52 @@ func (f *BasicFilesystem) GetXattr(path string, xattrFilter XattrFilter) ([]prot
 	return res, nil
 }
 
-func getXattr(path, name string, buf []byte) (val []byte, rest []byte, err error) {
-	if len(buf) == 0 {
-		buf = make([]byte, 1024)
-	}
+var xattrBufPool = sync.Pool{
+	New: func() any { return make([]byte, 1024) },
+}
+
+func getXattr(path, name string) ([]byte, error) {
+	buf := xattrBufPool.Get().([]byte)
+	defer func() {
+		// Put the buffer back in the pool, or not if we're not supposed to
+		// (we returned it to the caller).
+		if buf != nil {
+			xattrBufPool.Put(buf)
+		}
+	}()
+
 	size, err := unix.Lgetxattr(path, name, buf)
 	if errors.Is(err, unix.ERANGE) {
 		// Buffer was too small. Figure out how large it needs to be, and
 		// allocate.
 		size, err = unix.Lgetxattr(path, name, nil)
 		if err != nil {
-			return nil, nil, fmt.Errorf("Lgetxattr %s %q: %w", path, name, err)
+			return nil, fmt.Errorf("Lgetxattr %s %q: %w", path, name, err)
 		}
 		if size > len(buf) {
+			xattrBufPool.Put(buf)
 			buf = make([]byte, size)
 		}
 		size, err = unix.Lgetxattr(path, name, buf)
 	}
 	if err != nil {
-		return nil, buf, fmt.Errorf("Lgetxattr %s %q: %w", path, name, err)
+		return nil, fmt.Errorf("Lgetxattr %s %q: %w", path, name, err)
+	}
+
+	if size >= len(buf)/4*3 {
+		// The buffer is adequately sized (at least three quarters of it is
+		// used), return it as-is.
+		val := buf[:size]
+		buf = nil // Don't put it back in the pool.
+		return val, nil
 	}
-	return buf[:size], buf[size:], nil
+
+	// The buffer is larger than required, copy the data to a new buffer of
+	// the correct size. This avoids having lots of 1024-sized allocations
+	// sticking around when 24 bytes or whatever would be enough.
+	val := make([]byte, size)
+	copy(val, buf)
+	return val, nil
 }
 
 func (f *BasicFilesystem) SetXattr(path string, xattrs []protocol.Xattr, xattrFilter XattrFilter) error {