From ec35a95318956aabb258868ece8d0b3e99d7feb6 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 25 Jul 2026 09:46:56 +0200 Subject: [PATCH] chore: slightly optimise rename detection (ref #10777) (#10819) Only run rename detection for new files. This skips an expensive check for all updates to existing files. The tradeoff is that we no longer immediately detect renames on top of another file as a rename -- this may instead become a copy+delete operation on the destination. --------- Signed-off-by: Jakob Borg --- lib/model/folder.go | 19 +++++++++++++------ lib/model/model_test.go | 17 ++++++++++------- lib/protocol/bep_fileinfo.go | 5 ++++- lib/scanner/walk.go | 6 ++++++ 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/model/folder.go b/lib/model/folder.go index 9f50fc80c..fb6051546 100644 --- a/lib/model/folder.go +++ b/lib/model/folder.go @@ -745,12 +745,19 @@ func (f *folder) scanSubdirsChangedAndNew(ctx context.Context, subDirs []string, switch f.Type { case config.FolderTypeReceiveOnly, config.FolderTypeReceiveEncrypted: default: - if nf, ok := f.findRename(ctx, res.File, batch); ok { - if ok, err := batch.Update(nf); err != nil { - return 0, err - } else if ok { - changes++ - batch.markDeleted(nf.Name) + // Rename detection is comparatively expensive, so only attempt + // it for files that appeared as new on disk during this scan. A + // rename that overwrites an existing file (the destination path + // already had an entry, so it scans as an update rather than a + // new file) is not optimised as a rename. + if res.File.New && res.File.Size > 0 { + if nf, ok := f.findRename(ctx, res.File, batch); ok { + if ok, err := batch.Update(nf); err != nil { + return 0, err + } else if ok { + changes++ + batch.markDeleted(nf.Name) + } } } } diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 1964c05ff..8cac582eb 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -3270,17 +3270,20 @@ func TestRenameSequenceOrder(t *testing.T) { t.Errorf("Unexpected count: %d != %d", count, numFiles) } - // Modify all the files, other than the ones we expect to rename + // Modify all the files other than the rename sources, whose content we + // keep intact so the renamed copies still match by block hash. for i := 0; i < numFiles; i++ { - if i == 3 || i == 17 || i == 16 || i == 4 { + if i == 3 || i == 16 { continue } v := fmt.Sprintf("%d", i) writeFile(t, ffs, v, []byte(v+"-new")) } - // Rename - must(t, ffs.Rename("3", "17")) - must(t, ffs.Rename("16", "4")) + // Rename to previously unseen names. Renaming onto an existing name is + // treated as an in-place update rather than a rename, since rename + // detection only runs for files that are new on disk. + must(t, ffs.Rename("3", "20")) + must(t, ffs.Rename("16", "21")) // Scan m.ScanFolders() @@ -3291,10 +3294,10 @@ func TestRenameSequenceOrder(t *testing.T) { it, errFn := m.LocalFilesSequenced("default", protocol.LocalDeviceID, 0) for i := range it { t.Log(i) - if i.FileName() == "17" { + if i.FileName() == "20" { firstExpectedSequence = i.SequenceNo() + 1 } - if i.FileName() == "4" { + if i.FileName() == "21" { secondExpectedSequence = i.SequenceNo() + 1 } if i.FileName() == "3" { diff --git a/lib/protocol/bep_fileinfo.go b/lib/protocol/bep_fileinfo.go index 329ce5a58..ad3d17e6c 100644 --- a/lib/protocol/bep_fileinfo.go +++ b/lib/protocol/bep_fileinfo.go @@ -146,6 +146,8 @@ type FileInfo struct { Deleted bool NoPermissions bool + New bool // set by the scanner when the file newly appeared on disk; no db / wire representation + truncated bool // was created from a truncated file info without blocks } @@ -233,7 +235,8 @@ func (f *FileInfo) LogAttr() slog.Attr { case FileInfoTypeFile: kind = "file" if !f.Deleted { - attrs = append(attrs, + attrs = append( + attrs, slog.Any("modified", f.ModTime()), slog.String("permissions", fmt.Sprintf("0%03o", f.Permissions)), slog.Int64("size", f.Size), diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go index 479beeb08..2b8bfb5d4 100644 --- a/lib/scanner/walk.go +++ b/lib/scanner/walk.go @@ -471,6 +471,8 @@ func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileIn f.RawBlockSize = int32(blockSize) l.Debugln(w, "checking:", f) + f.New = !hasCurFile + if hasCurFile { if curFile.IsEquivalentOptional(f, protocol.FileInfoComparison{ ModTimeWindow: w.ModTimeWindow, @@ -518,6 +520,8 @@ func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, f.NoPermissions = w.IgnorePerms l.Debugln(w, "checking:", f) + f.New = !hasCurFile + if hasCurFile { if curFile.IsEquivalentOptional(f, protocol.FileInfoComparison{ ModTimeWindow: w.ModTimeWindow, @@ -570,6 +574,8 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, info fs.FileIn f = w.updateFileInfo(f, curFile) l.Debugln(w, "checking:", f) + f.New = !hasCurFile + if hasCurFile { if curFile.IsEquivalentOptional(f, protocol.FileInfoComparison{ ModTimeWindow: w.ModTimeWindow,