* [PATCH] commit-reach: stop sorting in paint_down_to_common()
@ 2026-05-27 15:52 René Scharfe
2026-05-29 8:43 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: René Scharfe @ 2026-05-27 15:52 UTC (permalink / raw)
To: Git List
None of the three callers of paint_down_to_common() care about the order
of its result list: merge_bases_many() sorts it again after removing
stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()
throw the list away without even looking at it. So drop the unnecessary
commit_list_sort_by_date() call.
Signed-off-by: René Scharfe <l.s.r@web•de>
---
commit-reach.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/commit-reach.c b/commit-reach.c
index 5a52be90a6..056a7ed8d8 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -137,7 +137,6 @@ static int paint_down_to_common(struct repository *r,
}
clear_prio_queue(&queue);
- commit_list_sort_by_date(result);
return 0;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
2026-05-27 15:52 [PATCH] commit-reach: stop sorting in paint_down_to_common() René Scharfe
@ 2026-05-29 8:43 ` Jeff King
2026-05-29 15:32 ` René Scharfe
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2026-05-29 8:43 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
On Wed, May 27, 2026 at 05:52:17PM +0200, René Scharfe wrote:
> None of the three callers of paint_down_to_common() care about the order
> of its result list: merge_bases_many() sorts it again after removing
> stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()
> throw the list away without even looking at it. So drop the unnecessary
> commit_list_sort_by_date() call.
Seems like an easy win. If some of the callers do not even look at the
result, could we avoid building it at all in those cases (e.g., by
passing in a NULL result pointer)?
I guess there is not much to be gained, though. The result is a list of
merge bases, so it should usually be rather small. The benefit in your
patch is probably not performance, but just reducing the size of the
code.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
2026-05-29 8:43 ` Jeff King
@ 2026-05-29 15:32 ` René Scharfe
2026-05-29 19:04 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: René Scharfe @ 2026-05-29 15:32 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
On 5/29/26 10:43 AM, Jeff King wrote:
> On Wed, May 27, 2026 at 05:52:17PM +0200, René Scharfe wrote:
>
>> None of the three callers of paint_down_to_common() care about the order
>> of its result list: merge_bases_many() sorts it again after removing
>> stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()
>> throw the list away without even looking at it. So drop the unnecessary
>> commit_list_sort_by_date() call.
>
> Seems like an easy win. If some of the callers do not even look at the
> result, could we avoid building it at all in those cases (e.g., by
> passing in a NULL result pointer)?
Yes, at the cost of adding NULL checks to paint_down_to_common(). Which
is probably worth it.
> I guess there is not much to be gained, though. The result is a list of
> merge bases, so it should usually be rather small. The benefit in your
> patch is probably not performance, but just reducing the size of the
> code.
True. The list can be arbitrarily long, but should only contain a
handful commits in normal repos.
René
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
2026-05-29 15:32 ` René Scharfe
@ 2026-05-29 19:04 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2026-05-29 19:04 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
On Fri, May 29, 2026 at 05:32:58PM +0200, René Scharfe wrote:
> > Seems like an easy win. If some of the callers do not even look at the
> > result, could we avoid building it at all in those cases (e.g., by
> > passing in a NULL result pointer)?
>
> Yes, at the cost of adding NULL checks to paint_down_to_common(). Which
> is probably worth it.
Yeah. I thought it was only one line (where we append to "tail"), but
there are a couple spots where "result" is referenced directly.
Another fun fact: it looks like paint_down_to_common() makes sure to
clean up the output list before returning an error. But the callers do
so, too, which is redundant. That would go away if they just passed in
NULL. :)
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-29 19:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 15:52 [PATCH] commit-reach: stop sorting in paint_down_to_common() René Scharfe
2026-05-29 8:43 ` Jeff King
2026-05-29 15:32 ` René Scharfe
2026-05-29 19:04 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox