Re: [PATCH 1/3] commit-reach: deduplicate queue entries in paint_down_to_common
From: Jeff King <hidden>
Date: 2026-05-25 07:01:17
On Sun, May 24, 2026 at 05:42:18PM +0000, Kristofer Karlsson via GitGitGadget wrote:
+static void maybe_enqueue(struct prio_queue *queue, struct commit *c)
+{
+ if (c->object.flags & ENQUEUED)
+ return;
+ c->object.flags |= ENQUEUED;
+ prio_queue_put(queue, c);
+}OK, so we mark each commit with ENQUEUED when we queue it, and then...
quoted hunk ↗ jump to hunk
@@ -83,6 +92,8 @@ static int paint_down_to_common(struct repository *r, int flags; timestamp_t generation = commit_graph_generation(commit); + commit->object.flags &= ~ENQUEUED; +
...clear that when we pop it. But the loop may terminate early before popping everything, and we get to this cleanup code at the end: clear_prio_queue(&queue); When we drop all of those queue elements, they'll all be left with the ENQUEUED flag set. Should we clear those? The ahead_behind() variant doesn't have the same problem, because it uses PARENT2 to check for queueing, and then does: /* STALE is used here, PARENT2 is used by insert_no_dup(). */ repo_clear_commit_marks(r, PARENT2 | STALE); So it's cleaning up both flags, whereas paint_down_to_common() is already leaving the STALE flag set. I'm not sure how much that matters (or if it is even an intentional thing communicated to the caller). But now we'd be adding ENQUEUED. -Peff