On 6/23/2026 10:09 AM, Kristofer Karlsson wrote:

On Tue, 23 Jun 2026 at 15:50, Derrick Stolee [off-list ref] wrote:

quoted

How much of this data that you are passing into the method could be
state in the paint_queue struct? Could we have the paint_queue manage
all of the state necessary to make decisions around the walk
termination?

Good idea, I think adding last_gen to the struct is doable and makes it cleaner.
If needed we could also add the mb_flags there (but would be a followup patch)
Minor note: I renamed the struct to paint_state so that I could rename
the prio_queue to queue and not have "queue.queue" which felt
confusing in the code.

quoted

Or, could we do a peek into the queue to see the "top" commit, and
check if it is a finite commit or not? I know that 'last_gen' is
supposed to be the commit walked in the previous cycle, but it seems
that we only care about "the remaining commits are finite" as our
condition.

Yes, peeking into the queue would work too, but it would feel awkward,

  commit = prio_queue_peek();
  if (halt conditions) return NULL;
  prio_queue_get();

Good instinct to notice that peeking and getting from the same
method is awkward.

And if we get first, the condition is not valid - that said, it would be doable
to instead put the halt conditions _between_ popping the commit and
updating the counters. I am not sure how ugly or confusing it would be,
but I could add a comment to explain why that sequencing is important.
(Popping the commit and updating the counters may lead to temporary
0 counts, but then when we enqueue parents of the commits they
move away from the 0 anyway). It would become something like:

// dry-/pseudo-coded
  commit *paint_queue_pop() {
    commit = prio_queue_pop();
    if (!commit) return NULL;
    if (halt_condition(state, commit.generation)) return NULL;
    // important: don't decrement counters before checking the halt condition
    paint_count_update(state, commit->object.flags, -1);
    return commit;
  }

I think this would be an appropriate way to handle this. If we
pop and return NULL then it's ok that we removed data from the
queue because it shouldn't be reused.

Thanks,
-Stolee