@matheusbsbs / matheus-bagliano / commits / 3677773371

commit-reach: use heuristic in remove_redundant()

Reachability algorithms in commit-reach.c frequently benefit from using the first-parent history as a heuristic for satisfying reachability queries. The most obvious example was implemented in 4fbcca4e (commit-reach: make can_all_from_reach... linear, 2018-07-20). Update the walk in remove_redundant() to use this same heuristic. Here, we are walking starting at the parents of the input commits. Sort those parents and walk from the highest generation to lower. Each time, use the heuristic of searching the first parent history before continuing to expand the walk. The order in which we explore the commits matters, so update compare_commits_by_gen to break generation number ties with commit date. This has no effect when the commits are in a commit-graph file with corrected commit dates computed, but it will assist when the commits are in the region "above" the commit-graph with "infinite" generation number. Note that we cannot shift to use compare_commits_by_gen_then_commit_date as the method prototype is different. We use compare_commits_by_gen for QSORT() as opposed to as a priority function. The important piece is to ensure we short-circuit the walk when we find that there is a single non-redundant commit. This happens frequently when looking for merge-bases or comparing several tags with 'git merge-base --independent'. Use a new count 'count_still_independent' and if that hits 1 we can stop walking. To update 'count_still_independent' properly, we add use of the RESULT flag on the input commits. Then we can detect when we reach one of these commits and decrease the count. We need to remove the RESULT flag at that moment because we might re-visit that commit when popping the stack. We use the STALE flag to mark parents that have been added to the new walk_start list, but we need to clear that flag before we start walking so those flags don't halt our depth-first-search walk. On my copy of the Linux kernel repository, the performance of 'git merge-base --independent <all-tags>' goes from 1.1 seconds to 0.11 seconds. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Feb 19, 2021 at 12:34 UTC 36777733713afeee31e7cf2dbb6d6a3dac186a51

1 file changed +56 -16

commit-reach.c

+56 -16

@@ -29,6 +29,10 @@ static int compare_commits_by_gen(const void *_a, const void *_b)

29 return -1;

30 if (generation_a > generation_b)

31 return 1;

32 + if (a->date < b->date)

33 + return -1;

34 + if (a->date > b->date)

35 + return 1;

36 return 0;

37 }

@@ -228,11 +232,10 @@ static int remove_redundant_no_gen(struct repository *r,

232 static int remove_redundant_with_gen(struct repository *r,

233 struct commit **array, int cnt)

234 {

231 - int i, count_non_stale = 0;

235 + int i, count_non_stale = 0, count_still_independent = cnt;

236 timestamp_t min_generation = GENERATION_NUMBER_INFINITY;

237 struct commit **walk_start;

238 size_t walk_start_nr = 0, walk_start_alloc = cnt;

235 - struct prio_queue queue = { compare_commits_by_gen_then_commit_date };

239

240 ALLOC_ARRAY(walk_start, walk_start_alloc);

241

@@ -242,6 +245,7 @@ static int remove_redundant_with_gen(struct repository *r,

245 timestamp_t generation;

246

247 repo_parse_commit(r, array[i]);

248 + array[i]->object.flags |= RESULT;

249 parents = array[i]->parents;

250

251 while (parents) {

@@ -250,7 +254,6 @@ static int remove_redundant_with_gen(struct repository *r,

254 parents->item->object.flags |= STALE;

255 ALLOC_GROW(walk_start, walk_start_nr + 1, walk_start_alloc);

256 walk_start[walk_start_nr++] = parents->item;

253 - prio_queue_put(&queue, parents->item);

257 }

258 parents = parents->next;

259 }

@@ -261,26 +264,63 @@ static int remove_redundant_with_gen(struct repository *r,

264 min_generation = generation;

265 }

266

264 - /* push the STALE bits up to min generation */

265 - while (queue.nr) {

266 - struct commit_list *parents;

267 - struct commit *c = prio_queue_get(&queue);

267 + QSORT(walk_start, walk_start_nr, compare_commits_by_gen);

268

269 - repo_parse_commit(r, c);

269 + /* remove STALE bit for now to allow walking through parents */

270 + for (i = 0; i < walk_start_nr; i++)

271 + walk_start[i]->object.flags &= ~STALE;

272

271 - if (commit_graph_generation(c) < min_generation)

272 - continue;

273 + /*

274 + * Start walking from the highest generation. Hopefully, it will

275 + * find all other items during the first-parent walk, and we can

276 + * terminate early. Otherwise, we will do the same amount of work

277 + * as before.

278 + */

279 + for (i = walk_start_nr - 1; i >= 0 && count_still_independent > 1; i--) {

280 + /* push the STALE bits up to min generation */

281 + struct commit_list *stack = NULL;

282

274 - parents = c->parents;

275 - while (parents) {

276 - if (!(parents->item->object.flags & STALE)) {

277 - parents->item->object.flags |= STALE;

278 - prio_queue_put(&queue, parents->item);

283 + commit_list_insert(walk_start[i], &stack);

284 + walk_start[i]->object.flags |= STALE;

285 +

286 + while (stack) {

287 + struct commit_list *parents;

288 + struct commit *c = stack->item;

289 +

290 + repo_parse_commit(r, c);

291 +

292 + if (c->object.flags & RESULT) {

293 + c->object.flags &= ~RESULT;

294 + if (--count_still_independent <= 1)

295 + break;

296 }

280 - parents = parents->next;

297 +

298 + if (commit_graph_generation(c) < min_generation) {

299 + pop_commit(&stack);

300 + continue;

301 + }

302 +

303 + parents = c->parents;

304 + while (parents) {

305 + if (!(parents->item->object.flags & STALE)) {

306 + parents->item->object.flags |= STALE;

307 + commit_list_insert(parents->item, &stack);

308 + break;

309 + }

310 + parents = parents->next;

311 + }

312 +

313 + /* pop if all parents have been visited already */

314 + if (!parents)

315 + pop_commit(&stack);

316 }

317 + free_commit_list(stack);

318 }

319

320 + /* clear result */

321 + for (i = 0; i < cnt; i++)

322 + array[i]->object.flags &= ~RESULT;

323 +

324 /* rearrange array */

325 for (i = count_non_stale = 0; i < cnt; i++) {

326 if (!(array[i]->object.flags & STALE))