patchcommit-reach: stop sorting in paint_down_to_common()
4 messages between May 27, 2026 and May 29, 2026, from René Scharfe, Jeff King.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
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(-)
Show changes to commit-reach.c +0 −1
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
Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
On Wed, May 27, 2026 at 05:52:17PM +0200, René Scharfe wrote:
Show 5 quoted lines
> 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
Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
On 5/29/26 10:43 AM, Jeff King wrote:
Show 11 quoted lines
> 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é
Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
On Fri, May 29, 2026 at 05:32:58PM +0200, René Scharfe wrote:
Show 6 quoted lines
> > 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