aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Borg <jakob@kastelo.net>2023-12-04 12:48:17 +0100
committerGitHub <noreply@github.com>2023-12-04 12:48:17 +0100
commitc1ec9a8826f2999fa6686ddbd95fd70dbe9024b7 (patch)
tree2046eb30a75a485d747a6a21e4f4570c893c4be7
parent1625b448928038689addf2c559a80cc89a21c12c (diff)
downloadsyncthing-c1ec9a8826f2999fa6686ddbd95fd70dbe9024b7.tar.gz
syncthing-c1ec9a8826f2999fa6686ddbd95fd70dbe9024b7.zip
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)
-rw-r--r--lib/fs/basicfs_xattr_unix.go43
1 files changed, 34 insertions, 9 deletions
diff --git a/lib/fs/basicfs_xattr_unix.go b/lib/fs/basicfs_xattr_unix.go
index eb26e7725..f43445c87 100644
--- a/lib/fs/basicfs_xattr_unix.go
+++ b/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 {