From 5c02ce52d6d746270e05c2fe7bb3478d3d937f53 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 30 May 2018 09:18:12 -0700 Subject: [PATCH] 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 --- .../storage/dfs/DfsPackDescriptionTest.java | 20 ++++++++++++++++ .../internal/storage/dfs/DfsObjDatabase.java | 20 +++++++++++++++- .../storage/dfs/DfsPackDescription.java | 23 +++++++++++++++++-- 3 files changed, 60 insertions(+), 3 deletions(-) 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 6029e9cae..55e1a9c4c 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 ce5efd0a8..09d59376a 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 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. + *

+ * 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 packComparator) { + this.packComparator = packComparator; + } + /** {@inheritDoc} */ @Override public DfsReader newReader() { @@ -607,7 +625,7 @@ public abstract class DfsObjDatabase extends ObjectDatabase { Map reftables = reftableMap(old); List scanned = listPacks(); - Collections.sort(scanned, DfsPackDescription.objectLookupComparator()); + Collections.sort(scanned, packComparator); List newPacks = new ArrayList<>(scanned.size()); List 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 53140baf4..5a1ac02e2 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. *

- * 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 objectLookupComparator() { + return objectLookupComparator(PackSource.DEFAULT_COMPARATOR); + } + + /** + * Comparator for packs when looking up objects in indexes. + *

+ * 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 objectLookupComparator( + Comparator packSourceComparator) { return Comparator.comparing( - DfsPackDescription::getPackSource, PackSource.DEFAULT_COMPARATOR) + DfsPackDescription::getPackSource, packSourceComparator) .thenComparing((a, b) -> { PackSource as = a.getPackSource(); PackSource bs = b.getPackSource();