git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] pack-bitmap: add free function

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jun 7, 2018, 19:04 UTC
Message-ID
<b9be1d0371e5dc694e5e7ac88a900f985faabec8.1528397984.git.jonathantanmy@google.com>
In-Reply-To
<cover.1528397984.git.jonathantanmy@google.com>

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 <jonathantanmy@google.com>
---
 builtin/pack-objects.c |  1 +
 builtin/rev-list.c     |  2 ++
 pack-bitmap-write.c    |  4 ++++
 pack-bitmap.c          | 35 +++++++++++++++++++++++++++++------
 pack-bitmap.h          |  1 +
 5 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d064f944b..896e41300 100644
--- 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
index cce42ae1d..62776721f 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -521,6 +521,7 @@ 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 &&
@@ -528,6 +529,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			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
index 03e122563..7896fedd3 100644
--- 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
index 7795444b0..c06b19f49 100644
--- 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,11 +724,11 @@ 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
@@ -735,7 +736,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)
 	 * 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
index 19f70043a..4555907de 100644
--- 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);
-- 
2.17.0.768.g1526ddbba1.dirty
Previous: Jonathan TanNext: Jeff King
Message 2 of 9 in “Object store refactoring: make bitmap_git not global”
  1. 0/2 Object store refactoring: make bitmap_git not globalJonathan Tan, Jun 7, 2018
  2. 2/2 pack-bitmap: add free functionJonathan Tan, Jun 7, 2018
  3. Jeff KingJun 9, 2018
  4. 1/2 pack-bitmap: remove bitmap_git global variableJonathan Tan, Jun 7, 2018
  5. Jeff KingJun 9, 2018
  6. Jonathan TanJun 11, 2018
  7. Jeff KingJun 11, 2018
  8. Stefan BellerJun 12, 2018
  9. Brandon WilliamsJun 25, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.