ReachabilityChecker: Receive a Stream instead of a Collection

Preparatory change. Converting ObjectIds to RevCommits is potentially
expensive and in the incremental reachability check, it is probably not
required for all elements in the collection.

Pass a Stream to the reachability checker. In the follow up we make
the conversion from ObjectId to RevCommit in the stream (i.e. on
demand). This should reduce the latency of reachability checks over big
sets of references.

Change-Id: I9f310e331de5b0bf8de34143bd7dcd34316d2fba
Signed-off-by: Ivan Frade <ifrade@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/ReachabilityCheckerTestCase.java

@@ -47,6 +47,7 @@ import static org.junit.Assert.assertTrue;
 
 import java.util.Arrays;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
@@ -83,11 +84,11 @@ public abstract class ReachabilityCheckerTestCase
 		ReachabilityChecker checker = getChecker(repo);
 
 		assertReachable("reachable from one tip",
-				checker.areAllReachable(Arrays.asList(a), Arrays.asList(c2)));
+				checker.areAllReachable(Arrays.asList(a), Stream.of(c2)));
 		assertReachable("reachable from another tip",
-				checker.areAllReachable(Arrays.asList(a), Arrays.asList(b2)));
+				checker.areAllReachable(Arrays.asList(a), Stream.of(b2)));
 		assertReachable("reachable from itself",
-				checker.areAllReachable(Arrays.asList(a), Arrays.asList(b2)));
+				checker.areAllReachable(Arrays.asList(a), Stream.of(b2)));
 	}
 
 	@Test
@@ -104,13 +105,13 @@ public abstract class ReachabilityCheckerTestCase
 
 		assertReachable("reachable through one branch",
 				checker.areAllReachable(Arrays.asList(b1),
-						Arrays.asList(merge)));
+						Stream.of(merge)));
 		assertReachable("reachable through another branch",
 				checker.areAllReachable(Arrays.asList(c1),
-						Arrays.asList(merge)));
+						Stream.of(merge)));
 		assertReachable("reachable, before the branching",
 				checker.areAllReachable(Arrays.asList(a),
-						Arrays.asList(merge)));
+						Stream.of(merge)));
 	}
 
 	@Test
@@ -123,7 +124,7 @@ public abstract class ReachabilityCheckerTestCase
 		ReachabilityChecker checker = getChecker(repo);
 
 		assertUnreachable("unreachable from the future",
-				checker.areAllReachable(Arrays.asList(b2), Arrays.asList(b1)));
+				checker.areAllReachable(Arrays.asList(b2), Stream.of(b1)));
 	}
 
 	@Test
@@ -137,7 +138,7 @@ public abstract class ReachabilityCheckerTestCase
 		ReachabilityChecker checker = getChecker(repo);
 
 		assertUnreachable("unreachable from different branch",
-				checker.areAllReachable(Arrays.asList(c1), Arrays.asList(b2)));
+				checker.areAllReachable(Arrays.asList(c1), Stream.of(b2)));
 	}
 
 	@Test
@@ -152,7 +153,7 @@ public abstract class ReachabilityCheckerTestCase
 		ReachabilityChecker checker = getChecker(repo);
 
 		assertReachable("reachable with long chain in the middle", checker
-				.areAllReachable(Arrays.asList(root), Arrays.asList(head)));
+				.areAllReachable(Arrays.asList(root), Stream.of(head)));
 	}
 
 	private static void assertReachable(String msg,
@@ -164,5 +165,4 @@ public abstract class ReachabilityCheckerTestCase
 			Optional<RevCommit> result) {
 		assertTrue(msg, result.isPresent());
 	}
-
 }

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

@@ -45,8 +45,10 @@ package org.eclipse.jgit.revwalk;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -88,7 +90,7 @@ class BitmappedReachabilityChecker implements ReachabilityChecker {
 	 */
 	@Override
 	public Optional<RevCommit> areAllReachable(Collection<RevCommit> targets,
-			Collection<RevCommit> starters) throws MissingObjectException,
+			Stream<RevCommit> starters) throws MissingObjectException,
 			IncorrectObjectTypeException, IOException {
 		BitmapCalculator calculator = new BitmapCalculator(walk);
 
@@ -105,8 +107,9 @@ class BitmappedReachabilityChecker implements ReachabilityChecker {
 		 * walk.reset() could start to take too much time.
 		 */
 		List<RevCommit> remainingTargets = new ArrayList<>(targets);
-		for (RevCommit starter : starters) {
-			BitmapBuilder starterBitmap = calculator.getBitmap(starter,
+		Iterator<RevCommit> it = starters.iterator();
+		while (it.hasNext()) {
+			BitmapBuilder starterBitmap = calculator.getBitmap(it.next(),
 					NullProgressMonitor.INSTANCE);
 			remainingTargets.removeIf(starterBitmap::contains);
 			if (remainingTargets.isEmpty()) {

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

@@ -44,7 +44,9 @@ package org.eclipse.jgit.revwalk;
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -75,7 +77,7 @@ class PedestrianReachabilityChecker implements ReachabilityChecker {
 
 	@Override
 	public Optional<RevCommit> areAllReachable(Collection<RevCommit> targets,
-			Collection<RevCommit> starters)
+			Stream<RevCommit> starters)
 					throws MissingObjectException, IncorrectObjectTypeException,
 					IOException {
 		walk.reset();
@@ -87,8 +89,9 @@ class PedestrianReachabilityChecker implements ReachabilityChecker {
 			walk.markStart(target);
 		}
 
-		for (RevCommit starter : starters) {
-			walk.markUninteresting(starter);
+		Iterator<RevCommit> iterator = starters.iterator();
+		while (iterator.hasNext()) {
+			walk.markUninteresting(iterator.next());
 		}
 
 		return Optional.ofNullable(walk.next());

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

@@ -45,6 +45,7 @@ package org.eclipse.jgit.revwalk;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -82,9 +83,43 @@ public interface ReachabilityChecker {
 	 * @throws IOException
 	 *             if any of the underlying indexes or readers can not be
 	 *             opened.
+	 *
+	 * @deprecated see {{@link #areAllReachable(Collection, Stream)}
+	 */
+	@Deprecated
+	default Optional<RevCommit> areAllReachable(Collection<RevCommit> targets,
+                       Collection<RevCommit> starters) throws MissingObjectException,
+			IncorrectObjectTypeException, IOException {
+		return areAllReachable(targets, starters.stream());
+	}
+
+	/**
+	 * Check if all targets are reachable from the {@code starter} commits.
+	 * <p>
+	 * Caller should parse the objectIds (preferably with
+	 * {@code walk.parseCommit()} and handle missing/incorrect type objects
+	 * before calling this method.
+	 *
+	 * @param targets
+	 *            commits to reach.
+	 * @param starters
+	 *            known starting points.
+	 * @return An unreachable target if at least one of the targets is
+	 *         unreachable. An empty optional if all targets are reachable from
+	 *         the starters.
+	 *
+	 * @throws MissingObjectException
+	 *             if any of the incoming objects doesn't exist in the
+	 *             repository.
+	 * @throws IncorrectObjectTypeException
+	 *             if any of the incoming objects is not a commit or a tag.
+	 * @throws IOException
+	 *             if any of the underlying indexes or readers can not be
+	 *             opened.
+	 * @since 5.6
 	 */
 	Optional<RevCommit> areAllReachable(Collection<RevCommit> targets,
-			Collection<RevCommit> starters)
+			Stream<RevCommit> starters)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			IOException;
 }

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -2029,7 +2029,7 @@ public class UploadPack {
 			List<RevCommit> reachableCommits = refsToRevCommits(walk,
 					sortedVisibleRefs);
 			Optional<RevCommit> unreachable = reachabilityChecker
-					.areAllReachable(wantsAsCommits, reachableCommits);
+					.areAllReachable(wantsAsCommits, reachableCommits.stream());
 			if (unreachable.isPresent()) {
 				throw new WantNotValidException(unreachable.get());
 			}