UploadPack: Prioritize references for non-advertised wants checks

UploadPack needs to check if object ids that weren't advertised before
are reachable from the references visible to the user. In the
bitmap-based reachability check, this is done incrementally: checking
against one reference, if anything remaining adding a second and so on.
It is more efficient to check first more common references (e.g. refs/heads/*)

Sort the references for the reachability checker. This should solve the
connectivity earlier and require less bitmap creation and less memory.

Change-Id: I48ac10d71e29fab2d346479802401eaea4aacb5c
Signed-off-by: Ivan Frade <ifrade@google.com>
This commit is contained in:
Ivan Frade 2019-11-12 14:33:29 -08:00
parent fa1566f40c
commit 2ff0c0abaa
1 changed files with 44 additions and 7 deletions

View File

@ -85,6 +85,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.eclipse.jgit.annotations.NonNull;
@ -1873,8 +1874,7 @@ public static final class ReachableCommitRequestValidator
@Override
public void checkWants(UploadPack up, List<ObjectId> wants)
throws PackProtocolException, IOException {
checkNotAdvertisedWants(up, wants,
refIdSet(up.getAdvertisedRefs().values()));
checkNotAdvertisedWants(up, wants, up.getAdvertisedRefs().values());
}
}
@ -1911,7 +1911,7 @@ public static final class ReachableCommitTipRequestValidator
public void checkWants(UploadPack up, List<ObjectId> wants)
throws PackProtocolException, IOException {
checkNotAdvertisedWants(up, wants,
refIdSet(up.getRepository().getRefDatabase().getRefs()));
up.getRepository().getRefDatabase().getRefs());
}
}
@ -1971,12 +1971,14 @@ private static void checkReachabilityByWalkingObjects(ObjectWalk walk,
}
private static void checkNotAdvertisedWants(UploadPack up,
List<ObjectId> notAdvertisedWants, Set<ObjectId> reachableFrom)
List<ObjectId> notAdvertisedWants, Collection<Ref> visibleRefs)
throws IOException {
ObjectReader reader = up.getRevWalk().getObjectReader();
try (RevWalk walk = new RevWalk(reader)) {
walk.setRetainBody(false);
Set<ObjectId> reachableFrom = refIdSet(visibleRefs);
// Missing "wants" throw exception here
List<RevObject> wantsAsObjs = objectIdsToRevObjects(walk,
notAdvertisedWants);
@ -2023,10 +2025,11 @@ private static void checkNotAdvertisedWants(UploadPack up,
ReachabilityChecker reachabilityChecker = walk
.createReachabilityChecker();
List<RevCommit> starters = objectIdsToRevCommits(walk,
reachableFrom);
List<Ref> sortedVisibleRefs = moreImportantRefsFirst(visibleRefs);
List<RevCommit> reachableCommits = refsToRevCommits(walk,
sortedVisibleRefs);
Optional<RevCommit> unreachable = reachabilityChecker
.areAllReachable(wantsAsCommits, starters);
.areAllReachable(wantsAsCommits, reachableCommits);
if (unreachable.isPresent()) {
throw new WantNotValidException(unreachable.get());
}
@ -2036,6 +2039,40 @@ private static void checkNotAdvertisedWants(UploadPack up,
}
}
private static List<Ref> moreImportantRefsFirst(
Collection<Ref> visibleRefs) {
Predicate<Ref> startsWithRefsHeads = ref -> ref.getName()
.startsWith(Constants.R_HEADS);
Predicate<Ref> startsWithRefsTags = ref -> ref.getName()
.startsWith(Constants.R_TAGS);
Predicate<Ref> allOther = ref -> !startsWithRefsHeads.test(ref)
&& !startsWithRefsTags.test(ref);
List<Ref> sorted = new ArrayList<>(visibleRefs.size());
sorted.addAll(filterRefByPredicate(visibleRefs, startsWithRefsHeads));
sorted.addAll(filterRefByPredicate(visibleRefs, startsWithRefsTags));
sorted.addAll(filterRefByPredicate(visibleRefs, allOther));
return sorted;
}
private static List<Ref> filterRefByPredicate(Collection<Ref> refs,
Predicate<Ref> predicate) {
return refs.stream().filter(predicate).collect(Collectors.toList());
}
private static List<RevCommit> refsToRevCommits(RevWalk walk,
List<Ref> refs) throws MissingObjectException, IOException {
List<ObjectId> objIds = refs.stream().map(
ref -> firstNonNull(ref.getPeeledObjectId(), ref.getObjectId()))
.collect(Collectors.toList());
return objectIdsToRevCommits(walk, objIds);
}
private static ObjectId firstNonNull(ObjectId one, ObjectId two) {
return one != null ? one : two;
}
// Resolve the ObjectIds into RevObjects. Any missing object raises an
// exception
private static List<RevObject> objectIdsToRevObjects(RevWalk walk,