Merge "Limit the range of commits for which bitmaps are created."

9 files changed, 159 insertions, 11 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java
index bbd41237e2..4f6d249c5f 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java
@@ -85,6 +85,7 @@ public class GcBasicPackingTest extends GcTestCase {
 		assertEquals(4, stats.numberOfLooseObjects);
 		assertEquals(0, stats.numberOfPackedObjects);
 		assertEquals(0, stats.numberOfPackFiles);
+		assertEquals(0, stats.numberOfBitmaps);
 	}
 
 	@Theory
@@ -102,6 +103,7 @@ public class GcBasicPackingTest extends GcTestCase {
 		assertEquals(0, stats.numberOfLooseObjects);
 		assertEquals(8, stats.numberOfPackedObjects);
 		assertEquals(1, stats.numberOfPackFiles);
+		assertEquals(2, stats.numberOfBitmaps);
 	}
 
 	@Theory
@@ -118,6 +120,7 @@ public class GcBasicPackingTest extends GcTestCase {
 		assertEquals(0, stats.numberOfLooseObjects);
 		assertEquals(4, stats.numberOfPackedObjects);
 		assertEquals(1, stats.numberOfPackFiles);
+		assertEquals(1, stats.numberOfBitmaps);
 
 		// Do the gc again and check that it hasn't changed anything
 		gc.gc();
@@ -125,6 +128,7 @@ public class GcBasicPackingTest extends GcTestCase {
 		assertEquals(0, stats.numberOfLooseObjects);
 		assertEquals(4, stats.numberOfPackedObjects);
 		assertEquals(1, stats.numberOfPackFiles);
+		assertEquals(1, stats.numberOfBitmaps);
 	}
 
 	@Theory
@@ -143,6 +147,7 @@ public class GcBasicPackingTest extends GcTestCase {
 		assertEquals(0, stats.numberOfLooseObjects);
 		assertEquals(8, stats.numberOfPackedObjects);
 		assertEquals(2, stats.numberOfPackFiles);
+		assertEquals(1, stats.numberOfBitmaps);
 	}
 
 	@Theory
@@ -220,6 +225,74 @@ public class GcBasicPackingTest extends GcTestCase {
 
 	}
 
+	@Test
+	public void testCommitRangeForBitmaps() throws Exception {
+		BranchBuilder bb1 = tr.branch("refs/heads/master");
+		bb1.commit().message("A1").add("A1", "A1").create();
+		bb1.commit().message("B1").add("B1", "B1").create();
+		bb1.commit().message("C1").add("C1", "C1").create();
+		BranchBuilder bb2 = tr.branch("refs/heads/working");
+		bb2.commit().message("A2").add("A2", "A2").create();
+		bb2.commit().message("B2").add("B2", "B2").create();
+		bb2.commit().message("C2").add("C2", "C2").create();
+
+		// Consider all commits. Since history isn't deep all commits are
+		// selected.
+		configureGcRange(gc, -1);
+		gc.gc();
+		assertEquals(6, gc.getStatistics().numberOfBitmaps);
+
+		// Range==0 means don't examine commit history, create bitmaps only for
+		// branch tips, C1 & C2.
+		configureGcRange(gc, 0);
+		gc.gc();
+		assertEquals(2, gc.getStatistics().numberOfBitmaps);
+
+		// Consider only the most recent commit (C2, which is also a branch
+		// tip).
+		configureGcRange(gc, 1);
+		gc.gc();
+		assertEquals(2, gc.getStatistics().numberOfBitmaps);
+
+		// Consider only the two most recent commits, C2 & B2. C1 gets included
+		// too since it is a branch tip.
+		configureGcRange(gc, 2);
+		gc.gc();
+		assertEquals(3, gc.getStatistics().numberOfBitmaps);
+
+		// Consider C2 & B2 & A2. C1 gets included too since it is a branch tip.
+		configureGcRange(gc, 3);
+		gc.gc();
+		assertEquals(4, gc.getStatistics().numberOfBitmaps);
+
+		// Consider C2 & B2 & A2 & C1.
+		configureGcRange(gc, 4);
+		gc.gc();
+		assertEquals(4, gc.getStatistics().numberOfBitmaps);
+
+		// Consider C2 & B2 & A2 & C1 & B1.
+		configureGcRange(gc, 5);
+		gc.gc();
+		assertEquals(5, gc.getStatistics().numberOfBitmaps);
+
+		// Consider all six commits.
+		configureGcRange(gc, 6);
+		gc.gc();
+		assertEquals(6, gc.getStatistics().numberOfBitmaps);
+
+		// Input is out of range but should be capped to the total number of
+		// commits.
+		configureGcRange(gc, 1000);
+		gc.gc();
+		assertEquals(6, gc.getStatistics().numberOfBitmaps);
+	}
+
+	private void configureGcRange(GC myGc, int range) {
+		PackConfig pconfig = new PackConfig(repo);
+		pconfig.setBitmapCommitRange(range);
+		myGc.setPackConfig(pconfig);
+	}
+
 	private void configureGc(GC myGc, boolean aggressive) {
 		PackConfig pconfig = new PackConfig(repo);
 		if (aggressive) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
index c5723c0594..e7005c247e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
@@ -877,6 +877,11 @@ public class GC {
 		 */
 		public long numberOfPackedRefs;
 
+		/**
+		 * The number of bitmaps in the bitmap indices.
+		 */
+		public long numberOfBitmaps;
+
 		public String toString() {
 			final StringBuilder b = new StringBuilder();
 			b.append("numberOfPackedObjects=").append(numberOfPackedObjects); //$NON-NLS-1$
@@ -886,15 +891,15 @@ public class GC {
 			b.append(", numberOfPackedRefs=").append(numberOfPackedRefs); //$NON-NLS-1$
 			b.append(", sizeOfLooseObjects=").append(sizeOfLooseObjects); //$NON-NLS-1$
 			b.append(", sizeOfPackedObjects=").append(sizeOfPackedObjects); //$NON-NLS-1$
+			b.append(", numberOfBitmaps=").append(numberOfBitmaps); //$NON-NLS-1$
 			return b.toString();
 		}
 	}
 
 	/**
-	 * Returns the number of objects stored in pack files. If an object is
-	 * contained in multiple pack files it is counted as often as it occurs.
+	 * Returns information about objects and pack files for a FileRepository.
 	 *
-	 * @return the number of objects stored in pack files
+	 * @return information about objects and pack files for a FileRepository
 	 * @throws IOException
 	 */
 	public RepoStatistics getStatistics() throws IOException {
@@ -904,6 +909,8 @@ public class GC {
 			ret.numberOfPackedObjects += f.getIndex().getObjectCount();
 			ret.numberOfPackFiles++;
 			ret.sizeOfPackedObjects += f.getPackFile().length();
+			if (f.getBitmapIndex() != null)
+				ret.numberOfBitmaps += f.getBitmapIndex().getBitmapCount();
 		}
 		File objDir = repo.getObjectsDirectory();
 		String[] fanout = objDir.list();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndex.java
index ae4de84a07..e743cb4aff 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndex.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndex.java
@@ -193,4 +193,11 @@ public abstract class PackBitmapIndex {
 	 *         pack that this index was generated from.
 	 */
 	public abstract int getObjectCount();
+
+	/**
+	 * Returns the number of bitmaps in this bitmap index.
+	 *
+	 * @return the number of bitmaps in this bitmap index.
+	 */
+	public abstract int getBitmapCount();
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexBuilder.java
index 93f891864a..3506953f14 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexBuilder.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexBuilder.java
@@ -253,7 +253,7 @@ public class PackBitmapIndexBuilder extends BasePackBitmapIndex {
 		return PackBitmapIndexV1.OPT_FULL;
 	}
 
-	/** @return the number of bitmaps. */
+	@Override
 	public int getBitmapCount() {
 		return getBitmaps().size();
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexRemapper.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexRemapper.java
index 6b96b07ed1..f2d9f6462e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexRemapper.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexRemapper.java
@@ -209,4 +209,10 @@ public class PackBitmapIndexRemapper extends PackBitmapIndex
 			return flags;
 		}
 	}
+
+	@Override
+	public int getBitmapCount() {
+		// The count is only useful for the end index, not the remapper.
+		return 0;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexV1.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexV1.java
index a38a26dec0..a7ab00db2d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexV1.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexV1.java
@@ -213,6 +213,11 @@ class PackBitmapIndexV1 extends BasePackBitmapIndex {
 	}
 
 	@Override
+	public int getBitmapCount() {
+		return bitmaps.size();
+	}
+
+	@Override
 	public boolean equals(Object o) {
 		// TODO(cranger): compare the pack checksum?
 		if (o instanceof PackBitmapIndexV1)
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
index 683d1cd6e9..cb69074539 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
@@ -2017,8 +2017,11 @@ public class PackWriter implements AutoCloseable {
 		PackWriterBitmapPreparer bitmapPreparer = new PackWriterBitmapPreparer(
 				reader, writeBitmaps, pm, stats.interestingObjects);
 
+		int commitRange = config.getBitmapCommitRange();
+		if (commitRange < 0)
+			commitRange = numCommits; // select from all commits
 		Collection<PackWriterBitmapPreparer.BitmapCommit> selectedCommits =
-				bitmapPreparer.doCommitSelection(numCommits);
+				bitmapPreparer.doCommitSelection(commitRange);
 
 		beginPhase(PackingPhase.BUILDING_BITMAPS, pm, selectedCommits.size());
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java
index 756d4b0005..efde4792ef 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java
@@ -109,13 +109,13 @@ class PackWriterBitmapPreparer {
 		this.bitmapIndex = new BitmapIndexImpl(bitmapRemapper);
 	}
 
-	Collection<BitmapCommit> doCommitSelection(int expectedNumCommits)
+	Collection<BitmapCommit> doCommitSelection(int commitRange)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			IOException {
 		pm.beginTask(JGitText.get().selectingCommits, ProgressMonitor.UNKNOWN);
 		RevWalk rw = new RevWalk(reader);
 		rw.setRetainBody(false);
-		WalkResult result = findPaths(rw, expectedNumCommits);
+		WalkResult result = findPaths(rw, commitRange);
 		pm.endTask();
 
 		int totCommits = result.commitsByOldest.length - result.commitStartPos;
@@ -215,7 +215,7 @@ class PackWriterBitmapPreparer {
 		return selections;
 	}
 
-	private WalkResult findPaths(RevWalk rw, int expectedNumCommits)
+	private WalkResult findPaths(RevWalk rw, int commitRange)
 			throws MissingObjectException, IOException {
 		BitmapBuilder reuseBitmap = commitBitmapIndex.newBitmapBuilder();
 		List<BitmapCommit> reuse = new ArrayList<BitmapCommit>();
@@ -256,11 +256,11 @@ class PackWriterBitmapPreparer {
 		}
 
 		// Update the paths from the wants and create a list of commits in
-		// reverse iteration order.
-		RevCommit[] commits = new RevCommit[expectedNumCommits];
+		// reverse iteration order for the desired commit range.
+		RevCommit[] commits = new RevCommit[commitRange];
 		int pos = commits.length;
 		RevCommit rc;
-		while ((rc = rw.next()) != null) {
+		while ((rc = rw.next()) != null && pos > 0) {
 			commits[--pos] = rc;
 			for (BitmapBuilder path : paths) {
 				if (path.contains(rc)) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java
index a8835b76c9..40309962c7 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java
@@ -138,6 +138,14 @@ public class PackConfig {
 	 */
 	public static final boolean DEFAULT_BUILD_BITMAPS = true;
 
+	/**
+	 * Default range of commits for which to create bitmaps: {@value}
+	 *
+	 * @see #setBitmapCommitRange(int)
+	 * @since 4.2
+	 */
+	public static final int DEFAULT_BITMAP_COMMIT_RANGE = -1;
+
 
 	private int compressionLevel = Deflater.DEFAULT_COMPRESSION;
 
@@ -169,6 +177,8 @@ public class PackConfig {
 
 	private boolean buildBitmaps = DEFAULT_BUILD_BITMAPS;
 
+	private int bitmapCommitRange = DEFAULT_BITMAP_COMMIT_RANGE;
+
 	private boolean cutDeltaChains;
 
 	/** Create a default configuration. */
@@ -222,6 +232,7 @@ public class PackConfig {
 		this.executor = cfg.executor;
 		this.indexVersion = cfg.indexVersion;
 		this.buildBitmaps = cfg.buildBitmaps;
+		this.bitmapCommitRange = cfg.bitmapCommitRange;
 		this.cutDeltaChains = cfg.cutDeltaChains;
 	}
 
@@ -684,6 +695,39 @@ public class PackConfig {
 	}
 
 	/**
+	 * Get the range of commits for which to build bitmaps. The range starts
+	 * from the most recent commit.
+	 *
+	 * A value of 0 creates bitmaps for only branch tips. A value of -1 creates
+	 * bitmaps spaced through the entire history of commits.
+	 *
+	 * Default setting: {@value #DEFAULT_BITMAP_COMMIT_RANGE}
+	 *
+	 * @return the range of commits for which to create bitmaps, starting with
+	 *         the most recent commit
+	 * @see PackIndexWriter
+	 * @since 4.2
+	 */
+	public int getBitmapCommitRange() {
+		return bitmapCommitRange;
+	}
+
+	/**
+	 * Set the range of commits for which to build bitmaps.
+	 *
+	 * Default setting: {@value #DEFAULT_BITMAP_COMMIT_RANGE}
+	 *
+	 * @param range
+	 *            the range of commits for which to create bitmaps, starting
+	 *            with the most recent commit
+	 * @see PackIndexWriter
+	 * @since 4.2
+	 */
+	public void setBitmapCommitRange(final int range) {
+		bitmapCommitRange = range;
+	}
+
+	/**
 	 * Update properties by setting fields from the configuration.
 	 *
 	 * If a property's corresponding variable is not defined in the supplied
@@ -718,6 +762,8 @@ public class PackConfig {
 		setCutDeltaChains(rc.getBoolean(
 				"pack", "cutdeltachains", getCutDeltaChains())); //$NON-NLS-1$ //$NON-NLS-2$
 		setBuildBitmaps(rc.getBoolean("pack", "buildbitmaps", isBuildBitmaps())); //$NON-NLS-1$ //$NON-NLS-2$
+		setBitmapCommitRange(
+				rc.getInt("pack", "bitmapcommitrange", getBitmapCommitRange())); //$NON-NLS-1$ //$NON-NLS-2$
 	}
 
 	public String toString() {
@@ -735,6 +781,7 @@ public class PackConfig {
 		b.append(", reuseObjects=").append(isReuseObjects()); //$NON-NLS-1$
 		b.append(", deltaCompress=").append(isDeltaCompress()); //$NON-NLS-1$
 		b.append(", buildBitmaps=").append(isBuildBitmaps()); //$NON-NLS-1$
+		b.append(", bitmapCommitRange=").append(getBitmapCommitRange()); //$NON-NLS-1$
 		return b.toString();
 	}
 }