PackWriterBitmapPreparer: do not include annotated tags in bitmap

The annotated tags should be excluded from the bitmap associated
with the heads-only packfile. However, this was not happening
because of the check of exclusion of the peeled object instead
of the objectId to be excluded from the bitmap.

Sample use-case:

refs/heads/main
  ^
  |
 commit1 <-- commit2 <- annotated-tag1 <- tag1
  ^
  |
 commit0

When creating a bitmap for the above commit graph, before this
change all the commits are included (3 bitmaps), which is
incorrect, because all commits reachable from annotated tags
should not be included.

The heads-only bitmap should include only commit0 and commit1
but because PackWriterBitPreparer was checking for the peeled
pointer of tag1 to be excluded (commit2) which was not found in
the list of tags to exclude (annotated-tag1), the commit2 was
included, even if it wasn't reachable only from the head.

Add an additional check for exclusion of the original objectId
for allowing the exclusion of annotated tags and their pointed
commits. Add one specific test associated with an annotated tag
for making sure that this use-case is covered also.

Example repository benchmark for measuring the improvement:
# refs: 400k (2k heads, 88k tags, 310k changes)
# objects: 11M (88k of them are annotate tags)
# packfiles: 2.7G

Before this change:
GC time: 5h
clone --bare time: 7 mins

After this change:
GC time: 20 mins
clone --bare time: 3 mins

Bug: 581267
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Change-Id: Iff2bfc6587153001837220189a120ead9ac649dc

2 files changed, 37 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java
index cc826c30bd..55dfa697bc 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java
@@ -83,6 +83,40 @@ public class GcCommitSelectionTest extends GcTestCase {
 	}
 
 	@Test
+	public void testBitmapDoesNotIncludeAnnotatedTags() throws Exception {
+		/*
+		 * Make sure that the bitmap generated for the following commit
+		 * graph does not include commit2 because it is not reachable by any
+		 * heads, despite being reachable from tag1 through the annotated-tag1.
+		 *
+		 * refs/heads/main
+		 *   ^
+		 *   |
+		 *  commit1 <-- commit2 <- annotated-tag1 <- tag1
+		 *   ^
+		 *   |
+		 *  commit0
+		 */
+		String mainBranch = "refs/heads/main";
+		BranchBuilder bb = tr.branch(mainBranch);
+
+		String commitMsg = "commit msg";
+		String fileBody = "file body";
+		String tagName = "tag1";
+		bb.commit().message(commitMsg + " 1").add("file1", fileBody).create();
+		RevCommit commit1 = bb.commit().message(commitMsg + " 2").add("file2", fileBody).create();
+		RevCommit commit2 = bb.commit().message(commitMsg + " 3").add("file3", fileBody).create();
+		tr.lightweightTag(tagName, tr.tag(tagName, commit2));
+		tr.branch(mainBranch).update(commit1);
+
+		gc.setExpireAgeMillis(0);
+		gc.gc();
+
+		// Create only 2 bitmaps, for commit0 and commit1, excluding commit2
+		assertEquals(2, gc.getStatistics().numberOfBitmaps);
+	}
+
+	@Test
 	public void testBitmapSpansWithMerges() throws Exception {
 		/*
 		 * Commits that are merged. Since 55 is in the oldest history it is
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 f1ede2acff..9f2b4d9c88 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
@@ -408,6 +408,9 @@ class PackWriterBitmapPreparer {
 		List<RevCommit> newWantsByNewest = new ArrayList<>(want.size());
 		Set<RevCommit> newWants = new HashSet<>(want.size());
 		for (AnyObjectId objectId : want) {
+			if(excludeFromBitmapSelection.contains(objectId)) {
+				continue;
+			}
 			RevObject ro = rw.peel(rw.parseAny(objectId));
 			if (!(ro instanceof RevCommit) || reuse.contains(ro)
 					|| excludeFromBitmapSelection.contains(ro)) {