Skip to content

Commit 49633dc

Browse files
ttaylorrgitster
ttaylorr
authored and
gitster
committed
pack-bitmap: build pseudo-merge bitmaps after regular bitmaps
When generating bitmaps, `bitmap_builder_init()` starts with an initial selection of commits to receive bitmap coverage, and then determines a set of "maximal" commits based on its input. Commit 089f751 (pack-bitmap-write: build fewer intermediate bitmaps, 2020-12-08) has extensive details, but the gist is as follows: Each selected commit starts with one commit_mask bit in its "commit mask" bitmap. Then, we walk the first-parent history in topological order and OR each commit's mask into its (first) parent. Whenever that OR results in the parent having more bits set, the child is deemed to be non-maximal, and the frontier is pushed further back along the first parent history. That approach works extremely well for ordinary selected commits, whose first-parent histories often describe real sharing between the bitmaps we are going to write. It struggles, however, to efficiently generate pseudo-merge bitmaps. Unlike ordinary commits for which the above algorithm is designed, pseudo-merges don't represent any "real" commit in history, just a grouping of non-bitmapped reference tips. In that sense, their first parent is just a part of a larger set, and treating them like ordinary selected commits imposes a significant slow-down when generating bitmaps with pseudo-merges enabled. Consider partitioning all non-bitmapped reference tips into eight individual pseudo-merges via the following configuration: [bitmapPseudoMerge "all"] pattern=refs/ threshold=now stableSize=10000000 maxMerges=8 , the cost of generating a bitmap from scratch rises significantly: +------------------+-----------------+---------------+---------------------+ | | no pseudo-merge | pseudo-merges | Delta | | | | (HEAD^) | | +------------------+-----------------+---------------+---------------------+ | elapsed | 294.1 s | 575.0 s | +280.9 s (+95.5%) | | cycles | 1,365.5 B | 2,686.9 B | +1,321.4 B (+96.8%) | | instructions | 1,389.8 B | 2,546.6 B | +1,156.8 B (+83.2%) | | CPI | 0.983 | 1.055 | +0.073 (+7.4%) | +------------------+-----------------+---------------+---------------------+ This is a particularly poor trade-off, because the time saved by these pseudo-merges during, e.g., $ git rev-list --count --all --objects --use-bitmap-index is only: $ hyperfine -L v true,false -n 'pseudo-merges: {v}' ' GIT_TEST_USE_PSEUDO_MERGES={v} git.compile rev-list --count \ --objects --all --use-bitmap-index ' Benchmark 1: pseudo-merges: true Time (mean ± σ): 2.613 s ± 0.012 s [User: 2.308 s, System: 0.305 s] Range (min … max): 2.594 s … 2.633 s 10 runs Benchmark 2: pseudo-merges: false Time (mean ± σ): 52.205 s ± 0.170 s [User: 51.500 s, System: 0.697 s] Range (min … max): 51.956 s … 52.458 s 10 runs Summary pseudo-merges: true ran 19.98 ± 0.11 times faster than pseudo-merges: false In other words, we pay a nearly ~5 minute penalty to generate pseudo-merge bitmaps, but only save ~50 seconds during traversal. The problem stems from injecting pseudo-merges into the bitmap builder as if they were normal commits. The maximal commit selection algorithm was simply not designed for that case, and performs predictably poorly. The only reason we reused the maximal commit selection routine for pseudo-merges alongside regular non-pseudo-merge commits is because we represent them both as commit objects (where the pseudo-merge commits just represent a made-up commit as opposed to one that actually exists in a repository's object store). Instead, build the regular selected commit bitmaps first, considering only non-pseudo-merge commits in `bitmap_builder_init()`. Once those bitmaps have been stored, build each pseudo-merge bitmap separately and attach its parent and object bitmaps to the corresponding pseudo-merge entry before writing the extension. This keeps the regular bitmap build shaped like the no-pseudo-merge case. The later pseudo-merge fill can still stop at stored selected ancestor bitmaps, so it does not have to rewalk each pseudo-merge closure from scratch. When an existing bitmap has the same pseudo-merge parent set, reuse and remap that whole pseudo-merge bitmap before falling back to fill_bitmap_commit(). This preserves the benefit of stable pseudo-merges while keeping the on-disk format and reader behavior unchanged. As a result, the overhead cost for generating pseudo-merges in the above configuration is much smaller: +------------------+-----------------+---------------+-------------------+ | | no pseudo-merge | pseudo-merges | Delta | | | | (HEAD) | | +------------------+-----------------+---------------+-------------------+ | elapsed | 294.1 s | 328.4 s | +34.3 s (+11.7%) | | cycles | 1,365.5 B | 1,529.3 B | +163.7 B (+12.0%) | | instructions | 1,389.8 B | 1,552.8 B | +163.0 B (+11.7%) | | CPI | 0.983 | 0.985 | +0.002 (+0.2%) | +------------------+-----------------+---------------+-------------------+ Recall that at the start of this series, generating reachability bitmaps took 612.5 seconds *without* pseudo-merges. With this commit, it is still ~46.38% *faster* to generate reachability bitmaps *with* pseudo-merges than it was to generate bitmaps wihtout them at the beginning of this series. The changes to implement this are mostly straightforward. We exclude pseudo-merge commits from the existing bitmap generation, and walk over them in a separate pass, by either reusing an existing on-disk pseudo-merge, or passing the pseudo-merge commit itself back to the existing routine in `fill_bitmap_commit()`. (Note that the routine to build pseudo-merge bitmaps is the same both before and after this change, the difference is only that we do not let psuedo-merges participate in determining the set of maximal commits.) The only wrinkle is that `fill_bitmap_commit()` must be taught to not expect that all tree objects have been parsed, which is the case for any portion of history reachable by one or more pseudo-merge(s), but not by any non-pseudo-merge commit selected for bitmapping. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent b04d266 commit 49633dc

1 file changed

Lines changed: 174 additions & 36 deletions

File tree

‎pack-bitmap-write.c‎

Lines changed: 174 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -446,13 +446,17 @@ static void bitmap_builder_init(struct bitmap_builder *bb,
446446
revs.topo_order = 1;
447447
revs.first_parent_only = 1;
448448

449-
for (i = 0; i < writer->selected_nr; i++) {
449+
for (i = 0; i < bitmap_writer_nr_selected_commits(writer); i++) {
450450
struct bitmapped_commit *bc = &writer->selected[i];
451451
struct bb_commit *ent = bb_data_at(&bb->data, bc->commit);
452452

453+
if (bc->pseudo_merge)
454+
BUG("unexpected pseudo-merge at %"PRIuMAX,
455+
(uintmax_t)i);
456+
453457
ent->selected = 1;
454458
ent->maximal = 1;
455-
ent->pseudo_merge = bc->pseudo_merge;
459+
ent->pseudo_merge = 0;
456460
ent->idx = i;
457461

458462
ent->commit_mask = bitmap_new();
@@ -618,6 +622,8 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
618622

619623
static int reused_bitmaps_nr;
620624
static int reused_pseudo_merge_bitmaps_nr;
625+
static int pseudo_merge_bitmap_nr;
626+
static int pseudo_merge_bitmap_parents;
621627

622628
static int fill_bitmap_commit_calls_nr;
623629
static int fill_bitmap_commit_found_ancestor_nr;
@@ -631,8 +637,12 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
631637
const uint32_t *mapping)
632638
{
633639
int found;
640+
int from_pseudo_merge = commit->object.flags & BITMAP_PSEUDO_MERGE;
634641
uint32_t pos;
635642

643+
if (ent->pseudo_merge)
644+
BUG("unexpected pseudo-merge commit in fill_bitmap_commit()");
645+
636646
fill_bitmap_commit_calls_nr++;
637647

638648
if (!ent->bitmap)
@@ -648,10 +658,7 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
648658
struct ewah_bitmap *old;
649659
struct bitmap *remapped = bitmap_new();
650660

651-
if (commit->object.flags & BITMAP_PSEUDO_MERGE)
652-
old = pseudo_merge_bitmap_for_commit(old_bitmap, c);
653-
else
654-
old = bitmap_for_commit(old_bitmap, c);
661+
old = bitmap_for_commit(old_bitmap, c);
655662
/*
656663
* If this commit has an old bitmap, then translate that
657664
* bitmap and add its bits to this one. No need to walk
@@ -660,10 +667,7 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
660667
if (old && !rebuild_bitmap(mapping, old, remapped)) {
661668
bitmap_or(ent->bitmap, remapped);
662669
bitmap_free(remapped);
663-
if (commit->object.flags & BITMAP_PSEUDO_MERGE)
664-
reused_pseudo_merge_bitmaps_nr++;
665-
else
666-
reused_bitmaps_nr++;
670+
reused_bitmaps_nr++;
667671
continue;
668672
}
669673
bitmap_free(remapped);
@@ -696,12 +700,32 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
696700
* walk ensures we cover all parents.
697701
*/
698702
if (!(c->object.flags & BITMAP_PSEUDO_MERGE)) {
703+
struct tree *tree;
704+
705+
if (from_pseudo_merge && !c->object.parsed) {
706+
/*
707+
* Commits reachable from selected
708+
* non-pseudo-merges are already parsed
709+
* by the regular bitmap build.
710+
*
711+
* However, pseudo-merge fills can also
712+
* reach commits that were not covered
713+
* there, so parse any such leftovers
714+
* before reading their tree or parents.
715+
*/
716+
if (repo_parse_commit(writer->repo, c))
717+
return -1;
718+
}
719+
699720
pos = find_object_pos(writer, &c->object.oid, &found);
700721
if (!found)
701722
return -1;
702723
bitmap_set(ent->bitmap, pos);
703-
prio_queue_put(tree_queue,
704-
repo_get_commit_tree(writer->repo, c));
724+
725+
tree = repo_get_commit_tree(writer->repo, c);
726+
if (!tree)
727+
return -1;
728+
prio_queue_put(tree_queue, tree);
705729
}
706730

707731
for (p = c->parents; p; p = p->next) {
@@ -738,6 +762,137 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
738762
return 0;
739763
}
740764

765+
static int reuse_pseudo_merge_bitmap(struct bitmap_index *old_bitmap,
766+
const uint32_t *mapping,
767+
struct commit *merge,
768+
struct ewah_bitmap **out)
769+
{
770+
struct ewah_bitmap *old;
771+
struct bitmap *remapped;
772+
773+
if (!old_bitmap || !mapping)
774+
return 0;
775+
776+
old = pseudo_merge_bitmap_for_commit(old_bitmap, merge);
777+
if (!old)
778+
return 0;
779+
780+
remapped = bitmap_new();
781+
if (rebuild_bitmap(mapping, old, remapped) < 0) {
782+
bitmap_free(remapped);
783+
return 0;
784+
}
785+
786+
*out = bitmap_to_ewah(remapped);
787+
bitmap_free(remapped);
788+
reused_pseudo_merge_bitmaps_nr++;
789+
return 1;
790+
}
791+
792+
static int build_pseudo_merge_bitmap(struct bitmap_writer *writer,
793+
struct bitmap_index *old_bitmap,
794+
const uint32_t *mapping,
795+
struct commit *merge,
796+
struct ewah_bitmap **out)
797+
{
798+
struct bb_commit ent = { 0 };
799+
struct prio_queue queue = { NULL };
800+
struct prio_queue tree_queue = { NULL };
801+
unsigned parents = commit_list_count(merge->parents);
802+
int ret;
803+
804+
ent.bitmap = bitmap_new();
805+
806+
pseudo_merge_bitmap_nr++;
807+
pseudo_merge_bitmap_parents += parents;
808+
809+
if (reuse_pseudo_merge_bitmap(old_bitmap, mapping, merge, out)) {
810+
ret = 0;
811+
goto done;
812+
}
813+
814+
ret = fill_bitmap_commit(writer, &ent, merge, &queue, &tree_queue,
815+
old_bitmap, mapping);
816+
817+
if (!ret)
818+
*out = bitmap_to_ewah(ent.bitmap);
819+
820+
done:
821+
bitmap_free(ent.bitmap);
822+
clear_prio_queue(&queue);
823+
clear_prio_queue(&tree_queue);
824+
825+
return ret;
826+
}
827+
828+
static int build_pseudo_merge_bitmaps(struct bitmap_writer *writer,
829+
struct bitmap_index *old_bitmap,
830+
const uint32_t *mapping,
831+
int *nr_stored)
832+
{
833+
size_t i = bitmap_writer_nr_selected_commits(writer);
834+
int ret = 0;
835+
836+
if (!writer->pseudo_merges_nr)
837+
return 0;
838+
839+
trace2_region_enter("pack-bitmap-write", "building_pseudo_merge_bitmaps",
840+
writer->repo);
841+
842+
for (; i < writer->selected_nr; i++) {
843+
struct bitmapped_commit *merge = &writer->selected[i];
844+
struct commit_list *p;
845+
struct bitmap *parents = bitmap_new();
846+
struct ewah_bitmap *objects = NULL;
847+
848+
if (!merge->pseudo_merge)
849+
BUG("found non-pseudo merge commit at %"PRIuMAX,
850+
(uintmax_t)i);
851+
852+
for (p = merge->commit->parents; p; p = p->next) {
853+
int found;
854+
uint32_t pos = find_object_pos(writer,
855+
&p->item->object.oid,
856+
&found);
857+
if (!found) {
858+
bitmap_free(parents);
859+
ret = -1;
860+
goto done;
861+
}
862+
bitmap_set(parents, pos);
863+
}
864+
865+
merge->pseudo_merge_parents = bitmap_to_ewah(parents);
866+
bitmap_free(parents);
867+
868+
if (build_pseudo_merge_bitmap(writer, old_bitmap, mapping,
869+
merge->commit, &objects) < 0) {
870+
ret = -1;
871+
goto done;
872+
}
873+
merge->bitmap = objects;
874+
875+
(*nr_stored)++;
876+
display_progress(writer->progress, *nr_stored);
877+
}
878+
879+
done:
880+
trace2_region_leave("pack-bitmap-write", "building_pseudo_merge_bitmaps",
881+
writer->repo);
882+
883+
trace2_data_intmax("pack-bitmap-write", writer->repo,
884+
"pseudo_merge_bitmap_nr",
885+
pseudo_merge_bitmap_nr);
886+
trace2_data_intmax("pack-bitmap-write", writer->repo,
887+
"building_bitmaps_pseudo_merge_reused",
888+
reused_pseudo_merge_bitmaps_nr);
889+
trace2_data_intmax("pack-bitmap-write", writer->repo,
890+
"pseudo_merge_bitmap_parents",
891+
pseudo_merge_bitmap_parents);
892+
893+
return ret;
894+
}
895+
741896
static void store_selected(struct bitmap_writer *writer,
742897
struct bb_commit *ent, struct commit *commit)
743898
{
@@ -821,6 +976,10 @@ int bitmap_writer_build(struct bitmap_writer *writer)
821976
bitmap_free(ent->bitmap);
822977
ent->bitmap = NULL;
823978
}
979+
if (closed &&
980+
build_pseudo_merge_bitmaps(writer, old_bitmap, mapping,
981+
&nr_stored) < 0)
982+
closed = 0;
824983
clear_prio_queue(&queue);
825984
clear_prio_queue(&tree_queue);
826985
bitmap_builder_clear(&bb);
@@ -831,9 +990,6 @@ int bitmap_writer_build(struct bitmap_writer *writer)
831990
writer->repo);
832991
trace2_data_intmax("pack-bitmap-write", writer->repo,
833992
"building_bitmaps_reused", reused_bitmaps_nr);
834-
trace2_data_intmax("pack-bitmap-write", writer->repo,
835-
"building_bitmaps_pseudo_merge_reused",
836-
reused_pseudo_merge_bitmaps_nr);
837993
trace2_data_intmax("pack-bitmap-write", writer->repo,
838994
"fill_bitmap_commit_calls_nr",
839995
fill_bitmap_commit_calls_nr);
@@ -1015,23 +1171,6 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
10151171

10161172
CALLOC_ARRAY(pseudo_merge_ofs, writer->pseudo_merges_nr);
10171173

1018-
for (i = 0; i < writer->pseudo_merges_nr; i++) {
1019-
struct bitmapped_commit *merge = &writer->selected[base + i];
1020-
struct commit_list *p;
1021-
struct bitmap *parents = bitmap_new();
1022-
1023-
if (!merge->pseudo_merge)
1024-
BUG("found non-pseudo merge commit at %"PRIuMAX, (uintmax_t)i);
1025-
1026-
for (p = merge->commit->parents; p; p = p->next)
1027-
bitmap_set(parents,
1028-
find_object_pos(writer, &p->item->object.oid,
1029-
NULL));
1030-
1031-
merge->pseudo_merge_parents = bitmap_to_ewah(parents);
1032-
bitmap_free(parents);
1033-
}
1034-
10351174
start = hashfile_total(f);
10361175

10371176
for (i = 0; i < writer->pseudo_merges_nr; i++) {
@@ -1040,14 +1179,13 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
10401179
if (!merge->pseudo_merge)
10411180
BUG("found non-pseudo merge commit at %"PRIuMAX, (uintmax_t)i);
10421181

1043-
if (!merge->pseudo_merge_parents)
1044-
BUG("missing pseudo-merge parents bitmap for commit %s",
1182+
if (!merge->pseudo_merge_parents || !merge->bitmap)
1183+
BUG("missing pseudo-merge bitmap for commit %s",
10451184
oid_to_hex(&merge->commit->object.oid));
10461185

10471186
pseudo_merge_ofs[i] = hashfile_total(f);
1048-
10491187
dump_bitmap(f, merge->pseudo_merge_parents);
1050-
dump_bitmap(f, writer->selected[base+i].write_as);
1188+
dump_bitmap(f, merge->bitmap);
10511189
}
10521190

10531191
next_ext = st_add(hashfile_total(f),

0 commit comments

Comments
 (0)