pack-bitmap: add free function

jonathantanmy · gitster · commit f3c23db2d7e7 · 2018-06-21T12:22:48.000-07:00
Add a function to free struct bitmap_index instances, and use it where
needed (except when rebuild_existing_bitmaps() is used, since it creates
references to the bitmaps within the struct bitmap_index passed to it).

Note that the hashes field in struct bitmap_index is not freed because
it points to another field within the same struct. The documentation for
that field has been updated to clarify that.

Signed-off-by: Jonathan Tan &lt;jonathantanmy@google.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
@@ -2945,6 +2945,7 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
 	}
 
 	traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap);
+	free_bitmap_index(bitmap_git);
 	return 0;
 }
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
@@ -521,13 +521,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 				if (max_count >= 0 && max_count < commit_count)
 					commit_count = max_count;
 				printf("%d\n", commit_count);
+				free_bitmap_index(bitmap_git);
 				return 0;
 			}
 		} else if (revs.max_count < 0 &&
 			   revs.tag_objects && revs.tree_objects && revs.blob_objects) {
 			struct bitmap_index *bitmap_git;
 			if ((bitmap_git = prepare_bitmap_walk(&revs))) {
 				traverse_bitmap_commit_list(bitmap_git, &show_object_fast);
+				free_bitmap_index(bitmap_git);
 				return 0;
 			}
 		}
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
@@ -367,6 +367,10 @@ void bitmap_writer_reuse_bitmaps(struct packing_data *to_pack)
 	writer.reused = kh_init_sha1();
 	rebuild_existing_bitmaps(bitmap_git, to_pack, writer.reused,
 				 writer.show_progress);
+	/*
+	 * NEEDSWORK: rebuild_existing_bitmaps() makes writer.reused reference
+	 * some bitmaps in bitmap_git, so we can't free the latter.
+	 */
 }
 
 static struct ewah_bitmap *find_reused_bitmap(const unsigned char *sha1)
diff --git a/pack-bitmap.c b/pack-bitmap.c
@@ -66,7 +66,7 @@ struct bitmap_index {
 	/* Number of bitmapped commits */
 	uint32_t entry_count;
 
-	/* Name-hash cache (or NULL if not present). */
+	/* If not NULL, this is a name-hash cache pointing into map. */
 	uint32_t *hashes;
 
 	/*
@@ -350,6 +350,7 @@ struct bitmap_index *prepare_bitmap_git(void)
 	if (!open_pack_bitmap(bitmap_git) && !load_pack_bitmap(bitmap_git))
 		return bitmap_git;
 
+	free_bitmap_index(bitmap_git);
 	return NULL;
 }
 
@@ -690,7 +691,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)
 	/* try to open a bitmapped pack, but don't parse it yet
 	 * because we may not need to use it */
 	if (open_pack_bitmap(bitmap_git) < 0)
-		return NULL;
+		goto cleanup;
 
 	for (i = 0; i < revs->pending.nr; ++i) {
 		struct object *object = revs->pending.objects[i].item;
@@ -723,19 +724,19 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)
 	 * optimize here
 	 */
 	if (haves && !in_bitmapped_pack(bitmap_git, haves))
-		return NULL;
+		goto cleanup;
 
 	/* if we don't want anything, we're done here */
 	if (!wants)
-		return NULL;
+		goto cleanup;
 
 	/*
 	 * now we're going to use bitmaps, so load the actual bitmap entries
 	 * from disk. this is the point of no return; after this the rev_list
 	 * becomes invalidated and we must perform the revwalk through bitmaps
 	 */
 	if (!bitmap_git->loaded && load_pack_bitmap(bitmap_git) < 0)
-		return NULL;
+		goto cleanup;
 
 	object_array_clear(&revs->pending);
 
@@ -761,6 +762,10 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)
 
 	bitmap_free(haves_bitmap);
 	return bitmap_git;
+
+cleanup:
+	free_bitmap_index(bitmap_git);
+	return NULL;
 }
 
 int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
@@ -1001,7 +1006,7 @@ void test_bitmap_walk(struct rev_info *revs)
 	else
 		fprintf(stderr, "Mismatch!\n");
 
-	bitmap_free(result);
+	free_bitmap_index(bitmap_git);
 }
 
 static int rebuild_bitmap(uint32_t *reposition,
@@ -1093,3 +1098,21 @@ int rebuild_existing_bitmaps(struct bitmap_index *bitmap_git,
 	bitmap_free(rebuild);
 	return 0;
 }
+
+void free_bitmap_index(struct bitmap_index *b)
+{
+	if (!b)
+		return;
+
+	if (b->map)
+		munmap(b->map, b->map_size);
+	ewah_pool_free(b->commits);
+	ewah_pool_free(b->trees);
+	ewah_pool_free(b->blobs);
+	ewah_pool_free(b->tags);
+	kh_destroy_sha1(b->bitmaps);
+	free(b->ext_index.objects);
+	free(b->ext_index.hashes);
+	bitmap_free(b->result);
+	free(b);
+}
diff --git a/pack-bitmap.h b/pack-bitmap.h
@@ -48,6 +48,7 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
 				       uint32_t *entries, off_t *up_to);
 int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data *mapping,
 			     khash_sha1 *reused_bitmaps, int show_progress);
+void free_bitmap_index(struct bitmap_index *);
 
 void bitmap_writer_show_progress(int show);
 void bitmap_writer_set_checksum(unsigned char *sha1);

Original file line number	Diff line number	Diff line change
`@@ -2945,6 +2945,7 @@ static int get_object_list_from_bitmap(struct rev_info *revs)`
`2945`	`2945`	`}`
`2946`	`2946`
`2947`	`2947`	`traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap);`
	`2948`	`+ free_bitmap_index(bitmap_git);`
`2948`	`2949`	`return 0;`
`2949`	`2950`	`}`
`2950`	`2951`
Original file line number	Diff line number	Diff line change
`@@ -521,13 +521,15 @@ int cmd_rev_list(int argc, const char *argv, const char prefix)`
`521`	`521`	`if (max_count >= 0 && max_count < commit_count)`
`522`	`522`	`commit_count = max_count;`
`523`	`523`	`printf("%d\n", commit_count);`
	`524`	`+ free_bitmap_index(bitmap_git);`
`524`	`525`	`return 0;`
`525`	`526`	`}`
`526`	`527`	`} else if (revs.max_count < 0 &&`
`527`	`528`	`revs.tag_objects && revs.tree_objects && revs.blob_objects) {`
`528`	`529`	`struct bitmap_index *bitmap_git;`
`529`	`530`	`if ((bitmap_git = prepare_bitmap_walk(&revs))) {`
`530`	`531`	`traverse_bitmap_commit_list(bitmap_git, &show_object_fast);`
	`532`	`+ free_bitmap_index(bitmap_git);`
`531`	`533`	`return 0;`
`532`	`534`	`}`
`533`	`535`	`}`