Re: [PATCH 3/3] commit: use prio_queue_replace() in pop_most_recent_commit()
- From
René Scharfe <l.s.r@web.de>
- Date
- Jul 16, 2025, 09:38 UTC
- Message-ID
- <2cec288f-e632-431d-aba0-e98798b833ad@web.de>
- In-Reply-To
- <xmqqh5zdgp6r.fsf@gitster.g>
On 7/15/25 10:43 PM, Junio C Hamano wrote:
Show 19 quoted lines
> René Scharfe <l.s.r@web.de> 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?
Show 20 quoted lines
>
>> 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;
>> }