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 <jakob@kastelo.net>

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 {
+			// Rename detection is comparatively expensive, so only attempt
-				if ok, err := batch.Update(nf); err != nil {
+			// it for files that appeared as new on disk during this scan. A
-					return 0, err
+			// rename that overwrites an existing file (the destination path
-				} else if ok {
+			// already had an entry, so it scans as an update rather than a
-					changes++
+			// new file) is not optimised as a rename.
-					batch.markDeleted(nf.Name)
+			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)
+					}
 				}
 			}
 		}

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
+	// Rename to previously unseen names. Renaming onto an existing name is
-	must(t, ffs.Rename("3", "17"))
+	// treated as an in-place update rather than a rename, since rename
-	must(t, ffs.Rename("16", "4"))
+	// 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" {

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),

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,