Skip to content

Commit 3ae5fa0

Browse files
jonathantanmygitster
jonathantanmy
authored and
gitster
committed
pack-bitmap: remove bitmap_git global variable
Remove the bitmap_git global variable. Instead, generate on demand an instance of struct bitmap_index for code that needs to access it. This allows us significant control over the lifetime of instances of struct bitmap_index. In particular, packs can now be closed without worrying if an unnecessarily long-lived "pack" field in struct bitmap_index still points to it. The bitmap API is also clearer in that we need to first obtain a struct bitmap_index, then we use it. This patch raises two potential issues: (1) memory for the struct bitmap_index is allocated without being freed, and (2) prepare_bitmap_git() and prepare_bitmap_walk() can reuse a previously loaded bitmap. For (1), this will be dealt with in a subsequent patch in this patch set that also deals with freeing the contents of the struct bitmap_index (which were not freed previously, because they have global scope). For (2), current bitmap users only load the bitmap once at most (note that pack-objects can use bitmaps or write bitmaps, but not both at the same time), so support for reuse has no effect - and future users can pass around the struct bitmap_index * obtained if they need to do 2 or more things with the same bitmap. Helped-by: Stefan Beller <sbeller@google.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 53f9a3e commit 3ae5fa0

File tree

5 files changed

+201
-158
lines changed

5 files changed

+201
-158
lines changed

‎builtin/pack-objects.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2929,11 +2929,13 @@ static int pack_options_allow_reuse(void)
29292929

29302930
static int get_object_list_from_bitmap(struct rev_info *revs)
29312931
{
2932-
if (prepare_bitmap_walk(revs) < 0)
2932+
struct bitmap_index *bitmap_git;
2933+
if (!(bitmap_git = prepare_bitmap_walk(revs)))
29332934
return -1;
29342935

29352936
if (pack_options_allow_reuse() &&
29362937
!reuse_partial_packfile_from_bitmap(
2938+
bitmap_git,
29372939
&reuse_packfile,
29382940
&reuse_packfile_objects,
29392941
&reuse_packfile_offset)) {
@@ -2942,7 +2944,7 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
29422944
display_progress(progress_state, nr_result);
29432945
}
29442946

2945-
traverse_bitmap_commit_list(&add_object_entry_from_bitmap);
2947+
traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap);
29462948
return 0;
29472949
}
29482950

‎builtin/rev-list.c‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "reflog-walk.h"
1717
#include "oidset.h"
1818
#include "packfile.h"
19+
#include "object-store.h"
1920

2021
static const char rev_list_usage[] =
2122
"git rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -514,17 +515,19 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
514515
if (revs.count && !revs.left_right && !revs.cherry_mark) {
515516
uint32_t commit_count;
516517
int max_count = revs.max_count;
517-
if (!prepare_bitmap_walk(&revs)) {
518-
count_bitmap_commit_list(&commit_count, NULL, NULL, NULL);
518+
struct bitmap_index *bitmap_git;
519+
if ((bitmap_git = prepare_bitmap_walk(&revs))) {
520+
count_bitmap_commit_list(bitmap_git, &commit_count, NULL, NULL, NULL);
519521
if (max_count >= 0 && max_count < commit_count)
520522
commit_count = max_count;
521523
printf("%d\n", commit_count);
522524
return 0;
523525
}
524526
} else if (revs.max_count < 0 &&
525527
revs.tag_objects && revs.tree_objects && revs.blob_objects) {
526-
if (!prepare_bitmap_walk(&revs)) {
527-
traverse_bitmap_commit_list(&show_object_fast);
528+
struct bitmap_index *bitmap_git;
529+
if ((bitmap_git = prepare_bitmap_walk(&revs))) {
530+
traverse_bitmap_commit_list(bitmap_git, &show_object_fast);
528531
return 0;
529532
}
530533
}

‎pack-bitmap-write.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,11 +360,13 @@ static int date_compare(const void *_a, const void *_b)
360360

361361
void bitmap_writer_reuse_bitmaps(struct packing_data *to_pack)
362362
{
363-
if (prepare_bitmap_git() < 0)
363+
struct bitmap_index *bitmap_git;
364+
if (!(bitmap_git = prepare_bitmap_git()))
364365
return;
365366

366367
writer.reused = kh_init_sha1();
367-
rebuild_existing_bitmaps(to_pack, writer.reused, writer.show_progress);
368+
rebuild_existing_bitmaps(bitmap_git, to_pack, writer.reused,
369+
writer.show_progress);
368370
}
369371

370372
static struct ewah_bitmap *find_reused_bitmap(const unsigned char *sha1)

0 commit comments

Comments
 (0)