From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 30 May 2018 16:18:12 +0000 (-0700)
Subject: Allow overriding DfsPackDescription comparator for scanning packs
X-Git-Tag: v5.1.0.201808281540-m3~86
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fchanges%2F98%2F123698%2F3;p=jgit.git

Allow overriding DfsPackDescription comparator for scanning packs

Provide a factory for comparators that use the default heuristics except
with a different ordering of PackSources.

Change-Id: I0809b64deb3d0486040076946fdbdad650d69240
---

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackDescriptionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackDescriptionTest.java
index 6029e9cae8..55e1a9c4cc 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackDescriptionTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackDescriptionTest.java
@@ -46,8 +46,10 @@ package org.eclipse.jgit.internal.storage.dfs;
 import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.COMPACT;
 import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.GC;
 import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.GC_REST;
+import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.GC_TXN;
 import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.INSERT;
 import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.RECEIVE;
+import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.UNREACHABLE_GARBAGE;
 import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
 import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
 import static org.junit.Assert.assertEquals;
@@ -101,6 +103,24 @@ public final class DfsPackDescriptionTest {
 		assertComparesLessThan(DfsPackDescription.objectLookupComparator(), a, b);
 	}
 
+	@Test
+	public void objectLookupComparatorCustomPackSourceComparator()
+			throws Exception {
+		DfsPackDescription a = create(GC);
+
+		DfsPackDescription b = create(COMPACT);
+
+		assertComparesLessThan(DfsPackDescription.objectLookupComparator(), b, a);
+		assertComparesLessThan(
+				DfsPackDescription.objectLookupComparator(
+					new PackSource.ComparatorBuilder()
+						.add(GC)
+						.add(INSERT, RECEIVE, GC_REST, GC_TXN, UNREACHABLE_GARBAGE)
+						.add(COMPACT)
+						.build()),
+				a, b);
+	}
+
 	@Test
 	public void objectLookupComparatorGcFileSize() throws Exception {
 		// a is older and smaller.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
index ce5efd0a85..09d59376a0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
@@ -240,6 +240,8 @@ public abstract class DfsObjDatabase extends ObjectDatabase {
 
 	private DfsReaderOptions readerOptions;
 
+	private Comparator<DfsPackDescription> packComparator;
+
 	/**
 	 * Initialize an object database for our repository.
 	 *
@@ -253,6 +255,7 @@ public abstract class DfsObjDatabase extends ObjectDatabase {
 		this.repository = repository;
 		this.packList = new AtomicReference<>(NO_PACKS);
 		this.readerOptions = options;
+		this.packComparator = DfsPackDescription.objectLookupComparator();
 	}
 
 	/**
@@ -264,6 +267,21 @@ public abstract class DfsObjDatabase extends ObjectDatabase {
 		return readerOptions;
 	}
 
+	/**
+	 * Set the comparator used when searching for objects across packs.
+	 * <p>
+	 * An optimal comparator will find more objects without having to load large
+	 * idx files from storage only to find that they don't contain the object.
+	 * See {@link DfsPackDescription#objectLookupComparator()} for the default
+	 * heuristics.
+	 *
+	 * @param packComparator
+	 *            comparator.
+	 */
+	public void setPackComparator(Comparator<DfsPackDescription> packComparator) {
+		this.packComparator = packComparator;
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public DfsReader newReader() {
@@ -607,7 +625,7 @@ public abstract class DfsObjDatabase extends ObjectDatabase {
 		Map<DfsPackDescription, DfsReftable> reftables = reftableMap(old);
 
 		List<DfsPackDescription> scanned = listPacks();
-		Collections.sort(scanned, DfsPackDescription.objectLookupComparator());
+		Collections.sort(scanned, packComparator);
 
 		List<DfsPackFile> newPacks = new ArrayList<>(scanned.size());
 		List<DfsReftable> newReftables = new ArrayList<>(scanned.size());
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java
index 53140baf43..5a1ac02e25 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackDescription.java
@@ -72,13 +72,32 @@ public class DfsPackDescription {
 	 * with more recent modification dates before older packs, and packs with
 	 * fewer objects before packs with more objects.
 	 * <p>
-	 * Uses {@link PackSource#DEFAULT_COMPARATOR} for sorting sources.
+	 * Uses {@link PackSource#DEFAULT_COMPARATOR} for the portion of comparison
+	 * where packs are sorted by source.
 	 *
 	 * @return comparator.
 	 */
 	public static Comparator<DfsPackDescription> objectLookupComparator() {
+		return objectLookupComparator(PackSource.DEFAULT_COMPARATOR);
+	}
+
+	/**
+	 * Comparator for packs when looking up objects in indexes.
+	 * <p>
+	 * This comparator tries to position packs in the order readers should examine
+	 * them when looking for objects by SHA-1. The default tries to sort packs
+	 * with more recent modification dates before older packs, and packs with
+	 * fewer objects before packs with more objects.
+	 *
+	 * @param packSourceComparator
+	 *            comparator for the {@link PackSource}, used as the first step in
+	 *            comparison.
+	 * @return comparator.
+	 */
+	public static Comparator<DfsPackDescription> objectLookupComparator(
+			Comparator<PackSource> packSourceComparator) {
 		return Comparator.comparing(
-					DfsPackDescription::getPackSource, PackSource.DEFAULT_COMPARATOR)
+					DfsPackDescription::getPackSource, packSourceComparator)
 			.thenComparing((a, b) -> {
 				PackSource as = a.getPackSource();
 				PackSource bs = b.getPackSource();