@matheusbsbs / matheus-bagliano / commits / 45f4eeb291

pack-bitmap-write: relax unique revwalk condition

The previous commits improved the bitmap computation process for very long, linear histories with many refs by removing quadratic growth in how many objects were walked. The strategy of computing "intermediate commits" using bitmasks for which refs can reach those commits partitioned the poset of reachable objects so each part could be walked exactly once. This was effective for linear histories. However, there was a (significant) drawback: wide histories with many refs had an explosion of memory costs to compute the commit bitmasks during the exploration that discovers these intermediate commits. Since these wide histories are unlikely to repeat walking objects, the benefit of walking objects multiple times was not expensive before. But now, the commit walk *before computing bitmaps* is incredibly expensive. In an effort to discover a happy medium, this change reduces the walk for intermediate commits to only the first-parent history. This focuses the walk on how the histories converge, which still has significant reduction in repeat object walks. It is still possible to create quadratic behavior in this version, but it is probably less likely in realistic data shapes. Here is some data taken on a fresh clone of the kernel: | runtime (sec) | peak heap (GB) | | | | | from | with | from | with | | scratch | existing | scratch | existing | -----------+---------+----------+---------+----------- original | 64.044 | 83.241 | 2.088 | 2.194 | last patch | 45.049 | 37.624 | 2.267 | 2.334 | this patch | 88.478 | 53.218 | 2.157 | 2.224 | Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Dec 8, 2020 at 17:05 UTC 45f4eeb2919e2c802a6c1f64a2b1c299f7434938

2 files changed +22 -25

pack-bitmap-write.c

+5 -9

@@ -199,7 +199,7 @@ static void bitmap_builder_init(struct bitmap_builder *bb,

199 {

200 struct rev_info revs;

201 struct commit *commit;

202 - unsigned int i, num_maximal;

202 + unsigned int i, num_maximal = 0;

203

204 memset(bb, 0, sizeof(*bb));

205 init_bb_data(&bb->data);

@@ -207,6 +207,7 @@ static void bitmap_builder_init(struct bitmap_builder *bb,

207 reset_revision_walk();

208 repo_init_revisions(writer->to_pack->repo, &revs, NULL);

209 revs.topo_order = 1;

210 + revs.first_parent_only = 1;

211

212 for (i = 0; i < writer->selected_nr; i++) {

213 struct commit *c = writer->selected[i].commit;

@@ -221,13 +222,12 @@ static void bitmap_builder_init(struct bitmap_builder *bb,

222

223 add_pending_object(&revs, &c->object, "");

224 }

224 - num_maximal = writer->selected_nr;

225

226 if (prepare_revision_walk(&revs))

227 die("revision walk setup failed");

228

229 while ((commit = get_revision(&revs))) {

230 - struct commit_list *p;

230 + struct commit_list *p = commit->parents;

231 struct bb_commit *c_ent;

232

233 parse_commit_or_die(commit);

@@ -235,16 +235,12 @@ static void bitmap_builder_init(struct bitmap_builder *bb,

235 c_ent = bb_data_at(&bb->data, commit);

236

237 if (c_ent->maximal) {

238 - if (!c_ent->selected) {

239 - bitmap_set(c_ent->commit_mask, num_maximal);

240 - num_maximal++;

241 - }

242 -

238 + num_maximal++;

239 ALLOC_GROW(bb->commits, bb->commits_nr + 1, bb->commits_alloc);

240 bb->commits[bb->commits_nr++] = commit;

241 }

242

247 - for (p = commit->parents; p; p = p->next) {

243 + if (p) {

244 struct bb_commit *p_ent = bb_data_at(&bb->data, p->item);

245 int c_not_p, p_not_c;

246

t/t5310-pack-bitmaps.sh

+17 -16

@@ -23,12 +23,12 @@ has_any () {

23 # To ensure the logic for "maximal commits" is exercised, make

24 # the repository a bit more complicated.

25 #

26 -# other master

26 +# other second

27 # * *

28 # (99 commits) (99 commits)

29 # * *

30 # |\ /|

31 -# | * octo-other octo-master * |

31 +# | * octo-other octo-second * |

32 # |/|\_________ ____________/|\|

33 # | \ \/ __________/ |

34 # | | ________/\ / |

@@ -43,23 +43,24 @@ has_any () {

43 # \|

44 # * (base)

45 #

46 +# We only push bits down the first-parent history, which

47 +# makes some of these commits unimportant!

48 +#

49 # The important part for the maximal commit algorithm is how

50 # the bitmasks are extended. Assuming starting bit positions

48 -# for master (bit 0) and other (bit 1), and some flexibility

49 -# in the order that merge bases are visited, the bitmasks at

50 -# the end should be:

51 +# for second (bit 0) and other (bit 1), the bitmasks at the

52 +# end should be:

53 #

52 -# master: 1 (maximal, selected)

54 +# second: 1 (maximal, selected)

55 # other: 01 (maximal, selected)

54 -# octo-master: 1

55 -# octo-other: 01

56 -# merge-right: 111 (maximal)

57 -# (l1): 111

58 -# (r1): 111

59 -# merge-left: 1101 (maximal)

60 -# (l2): 11111 (maximal)

61 -# (r2): 111101 (maximal)

62 -# (base): 1111111 (maximal)

56 +# (base): 11 (maximal)

57 +#

58 +# This complicated history was important for a previous

59 +# version of the walk that guarantees never walking a

60 +# commit multiple times. That goal might be important

61 +# again, so preserve this complicated case. For now, this

62 +# test will guarantee that the bitmaps are computed

63 +# correctly, even with the repeat calculations.

65 test_expect_success 'setup repo with moderate-sized history' '

66 test_commit_bulk --id=file 10 &&

@@ -114,7 +115,7 @@ test_expect_success 'full repack creates bitmaps' '

115 ls .git/objects/pack/ | grep bitmap >output &&

116 test_line_count = 1 output &&

117 grep "\"key\":\"num_selected_commits\",\"value\":\"106\"" trace &&

117 - grep "\"key\":\"num_maximal_commits\",\"value\":\"111\"" trace

118 + grep "\"key\":\"num_maximal_commits\",\"value\":\"107\"" trace

119 '

120

121 test_expect_success 'rev-list --test-bitmap verifies bitmaps' '