From 6a9716e8a146b55d2580d989c34c8cfbb086b45b Mon Sep 17 00:00:00 2001
From: greatroar <61184462+greatroar@users.noreply.github.com>
Date: Wed, 10 Nov 2021 19:56:29 +0100
Subject: [PATCH] lib/model: Return index from deviceActivity.leastBusy

This way, we don't need a second loop over the Availabilities to remove
the selected item.
---
 lib/model/deviceactivity.go      | 15 +++++++--------
 lib/model/deviceactivity_test.go | 28 ++++++++++++++--------------
 lib/model/folder_sendrecv.go     | 18 +++++-------------
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/lib/model/deviceactivity.go b/lib/model/deviceactivity.go
index 524ba8501..4685f020c 100644
--- a/lib/model/deviceactivity.go
+++ b/lib/model/deviceactivity.go
@@ -26,20 +26,19 @@ func newDeviceActivity() *deviceActivity {
 	}
 }
 
-func (m *deviceActivity) leastBusy(availability []Availability) (Availability, bool) {
+// Returns the index of the least busy device, or -1 if all are too busy.
+func (m *deviceActivity) leastBusy(availability []Availability) int {
 	m.mut.Lock()
 	low := 2<<30 - 1
-	found := false
-	var selected Availability
-	for _, info := range availability {
-		if usage := m.act[info.ID]; usage < low {
+	best := -1
+	for i := range availability {
+		if usage := m.act[availability[i].ID]; usage < low {
 			low = usage
-			selected = info
-			found = true
+			best = i
 		}
 	}
 	m.mut.Unlock()
-	return selected, found
+	return best
 }
 
 func (m *deviceActivity) using(availability Availability) {
diff --git a/lib/model/deviceactivity_test.go b/lib/model/deviceactivity_test.go
index 02d901080..d61228035 100644
--- a/lib/model/deviceactivity_test.go
+++ b/lib/model/deviceactivity_test.go
@@ -19,42 +19,42 @@ func TestDeviceActivity(t *testing.T) {
 	devices := []Availability{n0, n1, n2}
 	na := newDeviceActivity()
 
-	if lb, ok := na.leastBusy(devices); !ok || lb != n0 {
+	if lb := na.leastBusy(devices); lb != 0 {
 		t.Errorf("Least busy device should be n0 (%v) not %v", n0, lb)
 	}
-	if lb, ok := na.leastBusy(devices); !ok || lb != n0 {
+	if lb := na.leastBusy(devices); lb != 0 {
 		t.Errorf("Least busy device should still be n0 (%v) not %v", n0, lb)
 	}
 
-	lb, _ := na.leastBusy(devices)
-	na.using(lb)
-	if lb, ok := na.leastBusy(devices); !ok || lb != n1 {
+	lb := na.leastBusy(devices)
+	na.using(devices[lb])
+	if lb := na.leastBusy(devices); lb != 1 {
 		t.Errorf("Least busy device should be n1 (%v) not %v", n1, lb)
 	}
-	lb, _ = na.leastBusy(devices)
-	na.using(lb)
-	if lb, ok := na.leastBusy(devices); !ok || lb != n2 {
+	lb = na.leastBusy(devices)
+	na.using(devices[lb])
+	if lb := na.leastBusy(devices); lb != 2 {
 		t.Errorf("Least busy device should be n2 (%v) not %v", n2, lb)
 	}
 
-	lb, _ = na.leastBusy(devices)
-	na.using(lb)
-	if lb, ok := na.leastBusy(devices); !ok || lb != n0 {
+	lb = na.leastBusy(devices)
+	na.using(devices[lb])
+	if lb := na.leastBusy(devices); lb != 0 {
 		t.Errorf("Least busy device should be n0 (%v) not %v", n0, lb)
 	}
 
 	na.done(n1)
-	if lb, ok := na.leastBusy(devices); !ok || lb != n1 {
+	if lb := na.leastBusy(devices); lb != 1 {
 		t.Errorf("Least busy device should be n1 (%v) not %v", n1, lb)
 	}
 
 	na.done(n2)
-	if lb, ok := na.leastBusy(devices); !ok || lb != n1 {
+	if lb := na.leastBusy(devices); lb != 1 {
 		t.Errorf("Least busy device should still be n1 (%v) not %v", n1, lb)
 	}
 
 	na.done(n0)
-	if lb, ok := na.leastBusy(devices); !ok || lb != n0 {
+	if lb := na.leastBusy(devices); lb != 0 {
 		t.Errorf("Least busy device should be n0 (%v) not %v", n0, lb)
 	}
 }
diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go
index cb3971470..763f7efe7 100644
--- a/lib/model/folder_sendrecv.go
+++ b/lib/model/folder_sendrecv.go
@@ -1532,8 +1532,8 @@ loop:
 		// Select the least busy device to pull the block from. If we found no
 		// feasible device at all, fail the block (and in the long run, the
 		// file).
-		selected, found := activity.leastBusy(candidates)
-		if !found {
+		found := activity.leastBusy(candidates)
+		if found == -1 {
 			if lastError != nil {
 				state.fail(errors.Wrap(lastError, "pull"))
 			} else {
@@ -1542,7 +1542,9 @@ loop:
 			break
 		}
 
-		candidates = removeAvailability(candidates, selected)
+		selected := candidates[found]
+		candidates[found] = candidates[len(candidates)-1]
+		candidates = candidates[:len(candidates)-1]
 
 		// Fetch the block, while marking the selected device as in use so that
 		// leastBusy can select another device when someone else asks.
@@ -1804,16 +1806,6 @@ func (f *sendReceiveFolder) inConflict(current, replacement protocol.Vector) boo
 	return false
 }
 
-func removeAvailability(availabilities []Availability, availability Availability) []Availability {
-	for i := range availabilities {
-		if availabilities[i] == availability {
-			availabilities[i] = availabilities[len(availabilities)-1]
-			return availabilities[:len(availabilities)-1]
-		}
-	}
-	return availabilities
-}
-
 func (f *sendReceiveFolder) moveForConflict(name, lastModBy string, scanChan chan<- string) error {
 	if isConflict(name) {
 		l.Infoln("Conflict for", name, "which is already a conflict copy; not copying again.")