On 7/15/25 10:43 PM, Junio C Hamano wrote:
> René Scharfe <l.s.r@xxxxxx> writes:
>
>> diff --git a/commit.c b/commit.c
>> index 0200759aaa..8244221b30 100644
>> --- a/commit.c
>> +++ b/commit.c
>> @@ -742,17 +742,24 @@ void commit_list_sort_by_date(struct commit_list **list)
>> struct commit *pop_most_recent_commit(struct prio_queue *queue,
>> unsigned int mark)
>> {
>> - struct commit *ret = prio_queue_get(queue);
>> + struct commit *ret = prio_queue_peek(queue);
>> + int delete_pending = 1;
>
> Briefly I was puzzled by the name (I would have called first-parent
> since the logic was "we treat first parent specially by using
> replace instead of get/put"), but the variable signals "instead of
> get to remove the item from the queue, we just peeked, so we need to
> remove it later" with its name, which is understandable.
Indeed, we're just interested in the removal part of prio_queue_get()
here, as we have done the cheap half (the peeking) already. We don't
have a prio_queue_delete(). Adding one would perhaps add some clarity
here, but also widen the interface and probably not bring much of a
performance gain.
So perhaps calling the variable get_pending like the prio_queue_get()
that we end up invoking would reduce the initial puzzlement?
>
>> struct commit_list *parents = ret->parents;
>>
>> while (parents) {
>> struct commit *commit = parents->item;
>> if (!repo_parse_commit(the_repository, commit) && !(commit->object.flags & mark)) {
>> commit->object.flags |= mark;
>> - prio_queue_put(queue, commit);
>> + if (delete_pending)
>> + prio_queue_replace(queue, commit);
>> + else
>> + prio_queue_put(queue, commit);
>> + delete_pending = 0;
>> }
>> parents = parents->next;
>> }
>> + if (delete_pending)
>> + prio_queue_get(queue);
>> return ret;
>> }
