Browse Source

lib/protocol: Use BlocksHash to compare block lists when available (#6401)

This is an optimization for faster equal checks on block lists.
Jakob Borg 5 years ago
parent
commit
20aaa5927b
3 changed files with 64 additions and 5 deletions
  1. 2 2
      lib/model/folder_sendrecv.go
  2. 14 3
      lib/protocol/bep_extensions.go
  3. 48 0
      lib/protocol/protocol_test.go

+ 2 - 2
lib/model/folder_sendrecv.go

@@ -364,7 +364,7 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<-
 
 		case file.Type == protocol.FileInfoTypeFile:
 			curFile, hasCurFile := snap.Get(protocol.LocalDeviceID, file.Name)
-			if _, need := blockDiff(curFile.Blocks, file.Blocks); hasCurFile && len(need) == 0 {
+			if hasCurFile && file.BlocksEqual(curFile) {
 				// We are supposed to copy the entire file, and then fetch nothing. We
 				// are only updating metadata, so we don't actually *need* to make the
 				// copy.
@@ -460,7 +460,7 @@ nextFile:
 		// we can just do a rename instead.
 		key := string(fi.Blocks[0].Hash)
 		for i, candidate := range buckets[key] {
-			if protocol.BlocksEqual(candidate.Blocks, fi.Blocks) {
+			if candidate.BlocksEqual(fi) {
 				// Remove the candidate from the bucket
 				lidx := len(buckets[key]) - 1
 				buckets[key][i] = buckets[key][lidx]

+ 14 - 3
lib/protocol/bep_extensions.go

@@ -216,7 +216,7 @@ func (f FileInfo) isEquivalent(other FileInfo, modTimeWindow time.Duration, igno
 
 	switch f.Type {
 	case FileInfoTypeFile:
-		return f.Size == other.Size && ModTimeEqual(f.ModTime(), other.ModTime(), modTimeWindow) && (ignoreBlocks || BlocksEqual(f.Blocks, other.Blocks))
+		return f.Size == other.Size && ModTimeEqual(f.ModTime(), other.ModTime(), modTimeWindow) && (ignoreBlocks || f.BlocksEqual(other))
 	case FileInfoTypeSymlink:
 		return f.SymlinkTarget == other.SymlinkTarget
 	case FileInfoTypeDirectory:
@@ -249,9 +249,20 @@ func PermsEqual(a, b uint32) bool {
 	}
 }
 
-// BlocksEqual returns whether two slices of blocks are exactly the same hash
+// BlocksEqual returns true when the two files have identical block lists.
+func (f FileInfo) BlocksEqual(other FileInfo) bool {
+	// If both sides have blocks hashes then we can just compare those.
+	if len(f.BlocksHash) > 0 && len(other.BlocksHash) > 0 {
+		return bytes.Equal(f.BlocksHash, other.BlocksHash)
+	}
+
+	// Actually compare the block lists in full.
+	return blocksEqual(f.Blocks, other.Blocks)
+}
+
+// blocksEqual returns whether two slices of blocks are exactly the same hash
 // and index pair wise.
-func BlocksEqual(a, b []BlockInfo) bool {
+func blocksEqual(a, b []BlockInfo) bool {
 	if len(b) != len(a) {
 		return false
 	}

+ 48 - 0
lib/protocol/protocol_test.go

@@ -867,3 +867,51 @@ func TestDispatcherToCloseDeadlock(t *testing.T) {
 		t.Fatal("timed out before dispatcher loop terminated")
 	}
 }
+
+func TestBlocksEqual(t *testing.T) {
+	blocksOne := []BlockInfo{{Hash: []byte{1, 2, 3, 4}}}
+	blocksTwo := []BlockInfo{{Hash: []byte{5, 6, 7, 8}}}
+	hashOne := []byte{42, 42, 42, 42}
+	hashTwo := []byte{29, 29, 29, 29}
+
+	cases := []struct {
+		b1 []BlockInfo
+		h1 []byte
+		b2 []BlockInfo
+		h2 []byte
+		eq bool
+	}{
+		{blocksOne, hashOne, blocksOne, hashOne, true},  // everything equal
+		{blocksOne, hashOne, blocksTwo, hashTwo, false}, // nothing equal
+		{blocksOne, hashOne, blocksOne, nil, true},      // blocks compared
+		{blocksOne, nil, blocksOne, nil, true},          // blocks compared
+		{blocksOne, nil, blocksTwo, nil, false},         // blocks compared
+		{blocksOne, hashOne, blocksTwo, hashOne, true},  // hashes equal, blocks not looked at
+		{blocksOne, hashOne, blocksOne, hashTwo, false}, // hashes different, blocks not looked at
+		{blocksOne, hashOne, nil, nil, false},           // blocks is different from no blocks
+		{blocksOne, nil, nil, nil, false},               // blocks is different from no blocks
+		{nil, hashOne, nil, nil, true},                  // nil blocks are equal, even of one side has a hash
+	}
+
+	for _, tc := range cases {
+		f1 := FileInfo{Blocks: tc.b1, BlocksHash: tc.h1}
+		f2 := FileInfo{Blocks: tc.b2, BlocksHash: tc.h2}
+
+		if !f1.BlocksEqual(f1) {
+			t.Error("f1 is always equal to itself", f1)
+		}
+		if !f2.BlocksEqual(f2) {
+			t.Error("f2 is always equal to itself", f2)
+		}
+		if res := f1.BlocksEqual(f2); res != tc.eq {
+			t.Log("f1", f1.BlocksHash, f1.Blocks)
+			t.Log("f2", f2.BlocksHash, f2.Blocks)
+			t.Errorf("f1.BlocksEqual(f2) == %v but should be %v", res, tc.eq)
+		}
+		if res := f2.BlocksEqual(f1); res != tc.eq {
+			t.Log("f1", f1.BlocksHash, f1.Blocks)
+			t.Log("f2", f2.BlocksHash, f2.Blocks)
+			t.Errorf("f2.BlocksEqual(f1) == %v but should be %v", res, tc.eq)
+		}
+	}
+}