From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>, sbeller@google.com
Subject: [PATCH 2/2] pack-bitmap: add free function
Date: Thu, 7 Jun 2018 12:04:14 -0700 [thread overview]
Message-ID: <b9be1d0371e5dc694e5e7ac88a900f985faabec8.1528397984.git.jonathantanmy@google.com> (raw)
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
next prev parent reply other threads:[~2018-06-07 19:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-07 19:04 [PATCH 0/2] Object store refactoring: make bitmap_git not global Jonathan Tan
2018-06-07 19:04 ` [PATCH 1/2] pack-bitmap: remove bitmap_git global variable Jonathan Tan
2018-06-09 6:04 ` Jeff King
2018-06-11 18:50 ` Jonathan Tan
2018-06-11 21:10 ` Jeff King
2018-06-07 19:04 ` Jonathan Tan [this message]
2018-06-09 6:06 ` [PATCH 2/2] pack-bitmap: add free function Jeff King
2018-06-12 0:00 ` [PATCH 0/2] Object store refactoring: make bitmap_git not global Stefan Beller
2018-06-25 18:22 ` Brandon Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b9be1d0371e5dc694e5e7ac88a900f985faabec8.1528397984.git.jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=git@vger.kernel.org \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).