tree:<depth> should not traverse overly-deep trees

If we are traversing a tree which is too deep, then there is no need to
traverse the children. Skipping children is much faster than traversing
the possibly thousands of objects which are directly or indirectly
referenced by the tree.

Change-Id: I6d68cc1d35da48e3288b9cc80356a281ab36863d
Signed-off-by: Matthew DeVore <matvore@gmail.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java

@@ -45,6 +45,7 @@
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTag;
 import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.storage.pack.PackStatistics;
 import org.eclipse.jgit.transport.UploadPack.RequestPolicy;
 import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
@@ -76,6 +77,8 @@ public class UploadPackTest {
 
 	private TestRepository<InMemoryRepository> remote;
 
+	private PackStatistics stats;
+
 	@Before
 	public void setUp() throws Exception {
 		server = newRepo("server");
@@ -460,6 +463,7 @@ private ByteArrayInputStream uploadPackV2Setup(RequestPolicy requestPolicy,
 
 		ByteArrayOutputStream recv = new ByteArrayOutputStream();
 		up.upload(send, recv, null);
+		stats = up.getStatistics();
 
 		return new ByteArrayInputStream(recv.toByteArray());
 	}
@@ -1630,6 +1634,7 @@ public void testV2FetchFilterTreeDepth0() throws Exception {
 				.has(preparator.blobLowDepth.toObjectId()));
 		assertFalse(client.getObjectDatabase()
 				.has(preparator.blobHighDepth.toObjectId()));
+		assertEquals(1, stats.getTreesTraversed());
 	}
 
 	@Test
@@ -1651,6 +1656,7 @@ public void testV2FetchFilterTreeDepth1_serverHasBitmap() throws Exception {
 				.has(preparator.blobLowDepth.toObjectId()));
 		assertFalse(client.getObjectDatabase()
 				.has(preparator.blobHighDepth.toObjectId()));
+		assertEquals(1, stats.getTreesTraversed());
 	}
 
 	@Test
@@ -1668,6 +1674,7 @@ public void testV2FetchFilterTreeDepth2() throws Exception {
 				.has(preparator.blobLowDepth.toObjectId()));
 		assertFalse(client.getObjectDatabase()
 				.has(preparator.blobHighDepth.toObjectId()));
+		assertEquals(2, stats.getTreesTraversed());
 	}
 
 	/**

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java

@@ -2100,6 +2100,12 @@ private void addObject(
 	/**
 	 * Determines if the object should be omitted from the pack as a result of
 	 * its depth (probably because of the tree:<depth> filter).
+	 * <p>
+	 * Causes {@code walker} to skip traversing the current tree, which ought to
+	 * have just started traversal, assuming this method is called as soon as a
+	 * new depth is reached.
+	 * <p>
+	 * This method increments the {@code treesTraversed} statistic.
 	 *
 	 * @param obj
 	 *            the object to check whether it should be omitted.
@@ -2116,12 +2122,17 @@ private boolean depthSkip(@NonNull RevObject obj, ObjectWalk walker) {
 		// A blob is considered one level deeper than the tree that contains it.
 		if (obj.getType() == OBJ_BLOB) {
 			treeDepth++;
+		} else {
+			stats.treesTraversed++;
 		}
 
-		// TODO: Do not continue traversing the tree, since its children
-		// will also be too deep.
-		return filterSpec.getTreeDepthLimit() != -1 &&
-				treeDepth > filterSpec.getTreeDepthLimit();
+		if (filterSpec.getTreeDepthLimit() < 0 ||
+			treeDepth <= filterSpec.getTreeDepthLimit()) {
+			return false;
+		}
+
+		walker.skipTree();
+		return true;
 	}
 
 	// Adds the given object as an object to be packed, first performing

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java

@@ -389,6 +389,17 @@ public RevCommit next() throws MissingObjectException,
 		}
 	}
 
+	/**
+	 * Skips the current tree such that {@link #nextObject()} does not return
+	 * any objects inside it. This should be called right after
+	 * {@link #nextObject()} returns the tree.
+	 *
+	 * @since 5.4
+	 */
+	public void skipTree() {
+		currVisit.ptr = currVisit.buf.length;
+	}
+
 	/**
 	 * Pop the next most recent object.
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackStatistics.java

@@ -266,6 +266,9 @@ public static class Accumulator {
 		/** Time in ms spent writing the pack. */
 		public long timeWriting;
 
+		/** Number of trees traversed in the walk when writing the pack. */
+		public long treesTraversed;
+
 		/**
 		 * Statistics about each object type in the pack (commits, tags, trees
 		 * and blobs.)
@@ -585,6 +588,14 @@ public long getTimeWriting() {
 		return statistics.timeWriting;
 	}
 
+	/**
+	 * @return number of trees traversed in the walk when writing the pack.
+	 * @since 5.4
+	 */
+	public long getTreesTraversed() {
+		return statistics.treesTraversed;
+	}
+
 	/**
 	 * Get total time spent processing this pack.
 	 *