From f17a07ea70f04dfc19056b467ef56e43a04778f6 Mon Sep 17 00:00:00 2001
From: Taylor Blau <me@ttaylorr.com>
Date: Sat, 10 Dec 2016 18:04:03 -0700
Subject: [PATCH] lfs/tq: sort batches by descending object size

---
 lfs/transfer_queue.go       |  6 ++++++
 test/test-batch-transfer.sh | 37 +++++++++++++++++++++++++++++++++++++
 test/testhelpers.sh         | 18 ++++++++++++++++++
 3 files changed, 61 insertions(+)

diff --git a/lfs/transfer_queue.go b/lfs/transfer_queue.go
index 5a7a2f8e..fb9aa55f 100644
--- a/lfs/transfer_queue.go
+++ b/lfs/transfer_queue.go
@@ -1,6 +1,7 @@
 package lfs
 
 import (
+	"sort"
 	"sync"
 
 	"github.com/git-lfs/git-lfs/api"
@@ -238,6 +239,11 @@ func (q *TransferQueue) collectBatches() {
 			batch = append(batch, t)
 		}
 
+		// Before enqueuing the next batch, sort it's items by size
+		// (largest first) so that workers can process the largest
+		// transfers first.
+		sort.Sort(sort.Reverse(batch))
+
 		retries, err := q.enqueueAndCollectRetriesFor(batch)
 		if err != nil {
 			q.errorc <- err
diff --git a/test/test-batch-transfer.sh b/test/test-batch-transfer.sh
index 482727f8..52d1455a 100755
--- a/test/test-batch-transfer.sh
+++ b/test/test-batch-transfer.sh
@@ -64,3 +64,40 @@ begin_test "batch transfer"
   assert_pointer "master" "a.dat" "$contents_oid" 1
 )
 end_test
+
+begin_test "batch transfers occur in reverse order by size"
+(
+  set -e
+
+  reponame="batch-order-test"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  git lfs track "*.dat"
+  git add .gitattributes
+  git commit -m "initial commit"
+
+  small_contents="small"
+  small_oid="$(calc_oid "$small_contents")"
+  printf "$small_contents" > small.dat
+
+  bigger_contents="bigger"
+  bigger_oid="$(calc_oid "$bigger_contents")"
+  printf "$bigger_contents" > bigger.dat
+
+  git add *.dat
+  git commit -m "add small and large objects"
+
+  GIT_CURL_VERBOSE=1 git push origin master 2>&1 | tee push.log
+
+  batch="$(grep "{\"operation\":\"upload\"" push.log | head -1)"
+
+  pos_small="$(substring_position "$batch" "$small_oid")"
+  pos_large="$(substring_position "$batch" "$bigger_oid")"
+
+  # Assert that the the larger object shows up earlier in the batch than the
+  # smaller object
+  [ "$pos_large" -lt "$pos_small" ]
+)
+end_test
+
diff --git a/test/testhelpers.sh b/test/testhelpers.sh
index 1157edfa..fc0d702d 100644
--- a/test/testhelpers.sh
+++ b/test/testhelpers.sh
@@ -281,6 +281,24 @@ setup_remote_repo_with_file() {
   grep "master -> master" push.log
 }
 
+# substring_position returns the position of a substring in a 1-indexed search
+# space.
+#
+#     [ "$(substring_position "foo bar baz" "baz")" -eq "9" ]
+substring_position() {
+  local str="$1"
+  local substr="$2"
+
+  # 1) Print the string...
+  # 2) Remove the substring and everything after it
+  # 3) Count the number of characters (bytes) left, i.e., the offset of the
+  #    string we were looking for.
+
+  echo "$str" \
+    | sed "s/$substr.*$//" \
+    | wc -c
+}
+
 # setup initializes the clean, isolated environment for integration tests.
 setup() {
   cd "$ROOTDIR"