git-p4: reduce number of server queries for fetches

When fetching changes from a depot using a full client spec, there is no need to perform as many queries as there are top-level paths in the client spec. Instead we query all changes in chronological order, also getting rid of the need to sort the results and remove duplicates. Signed-off-by: Sam Hocevar <[email protected]> Signed-off-by: Luke Diamand <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
dscho · Dec 21, 2015 · 1f90a64 · 1f90a64
1 parent cbc6924
commit 1f90a64
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 24 deletions.
diff --git a/git-p4.py b/git-p4.py
@@ -822,39 +822,37 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
                 die("cannot use --changes-block-size with non-numeric revisions")
             block_size = None
 
-    # Accumulate change numbers in a dictionary to avoid duplicates
-    changes = {}
+    changes = []
 
-    for p in depotPaths:
-        # Retrieve changes a block at a time, to prevent running
-        # into a MaxResults/MaxScanRows error from the server.
+    # Retrieve changes a block at a time, to prevent running
+    # into a MaxResults/MaxScanRows error from the server.
 
-        while True:
-            cmd = ['changes']
+    while True:
+        cmd = ['changes']
 
-            if block_size:
-                end = min(changeEnd, changeStart + block_size)
-                revisionRange = "%d,%d" % (changeStart, end)
-            else:
-                revisionRange = "%s,%s" % (changeStart, changeEnd)
+        if block_size:
+            end = min(changeEnd, changeStart + block_size)
+            revisionRange = "%d,%d" % (changeStart, end)
+        else:
+            revisionRange = "%s,%s" % (changeStart, changeEnd)
 
+        for p in depotPaths:
             cmd += ["%s...@%s" % (p, revisionRange)]
 
-            for line in p4_read_pipe_lines(cmd):
-                changeNum = int(line.split(" ")[1])
-                changes[changeNum] = True
+        # Insert changes in chronological order
+        for line in reversed(p4_read_pipe_lines(cmd)):
+            changes.append(int(line.split(" ")[1]))
 
-            if not block_size:
-                break
+        if not block_size:
+            break
 
-            if end >= changeEnd:
-                break
+        if end >= changeEnd:
+            break
 
-            changeStart = end + 1
+        changeStart = end + 1
 
-    changelist = changes.keys()
-    changelist.sort()
-    return changelist
+    changes = sorted(changes)
+    return changes
 
 def p4PathStartsWith(path, prefix):
     # This method tries to remedy a potential mixed-case issue:

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
@@ -128,7 +128,7 @@ test_expect_success 'Create a repo with multiple depot paths' '
 	done
 '
 
-test_expect_failure 'Clone repo with multiple depot paths' '
+test_expect_success 'Clone repo with multiple depot paths' '
 	(
 		cd "$git" &&
 		git p4 clone --changes-block-size=4 //depot/pathA@all //depot/pathB@all \