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>

4 files changed, 49 insertions, 8 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java
index 6e510f677b..2a8b295d97 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/BitmappedReachabilityChecker.java
+++ b/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()) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianReachabilityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianReachabilityChecker.java
index bba3c5cff3..da9e75992f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PedestrianReachabilityChecker.java
+++ b/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());
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ReachabilityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ReachabilityChecker.java
index 2ed06d1769..6a9c641318 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ReachabilityChecker.java
+++ b/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;
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 5d952ff49b..a26d23d6db 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/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());
 			}