{"thread":{"id":"65701","subject":"[PATCH] commit-reach: stop sorting in paint_down_to_common()","startedAt":"2026-05-27T15:52:25Z","lastAt":"2026-05-29T19:04:55Z","messageCount":4,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"544175","messageId":"450163b0-82c8-4b57-baab-a269efe430aa@web.de","threadId":"65701","inReplyTo":null,"subject":"[PATCH] commit-reach: stop sorting in paint_down_to_common()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-05-27T15:52:17Z","receivedAt":"2026-05-27T15:52:25Z","isPatch":true,"body":"None of the three callers of paint_down_to_common() care about the order\nof its result list: merge_bases_many() sorts it again after removing\nstale items, remove_redundant_no_gen() and repo_in_merge_bases_many()\nthrow the list away without even looking at it.  So drop the unnecessary\ncommit_list_sort_by_date() call.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n commit-reach.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex 5a52be90a6..056a7ed8d8 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -137,7 +137,6 @@ static int paint_down_to_common(struct repository *r,\n \t}\n \n \tclear_prio_queue(&queue);\n-\tcommit_list_sort_by_date(result);\n \treturn 0;\n }\n \n-- \n2.54.0\n"},{"id":"544264","messageId":"20260529084325.GF1106035@coredump.intra.peff.net","threadId":"65701","inReplyTo":"450163b0-82c8-4b57-baab-a269efe430aa@web.de","subject":"Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-05-29T08:43:25Z","receivedAt":"2026-05-29T08:43:26Z","isPatch":true,"body":"On Wed, May 27, 2026 at 05:52:17PM +0200, René Scharfe wrote:\n\n> None of the three callers of paint_down_to_common() care about the order\n> of its result list: merge_bases_many() sorts it again after removing\n> stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()\n> throw the list away without even looking at it.  So drop the unnecessary\n> commit_list_sort_by_date() call.\n\nSeems like an easy win. If some of the callers do not even look at the\nresult, could we avoid building it at all in those cases (e.g., by\npassing in a NULL result pointer)?\n\nI guess there is not much to be gained, though. The result is a list of\nmerge bases, so it should usually be rather small. The benefit in your\npatch is probably not performance, but just reducing the size of the\ncode.\n\n-Peff\n"},{"id":"544271","messageId":"107314f5-0057-4ed3-9bee-9dca4f424bd1@web.de","threadId":"65701","inReplyTo":"20260529084325.GF1106035@coredump.intra.peff.net","subject":"Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-05-29T15:32:58Z","receivedAt":"2026-05-29T15:33:10Z","isPatch":true,"body":"On 5/29/26 10:43 AM, Jeff King wrote:\n> On Wed, May 27, 2026 at 05:52:17PM +0200, René Scharfe wrote:\n> \n>> None of the three callers of paint_down_to_common() care about the order\n>> of its result list: merge_bases_many() sorts it again after removing\n>> stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()\n>> throw the list away without even looking at it.  So drop the unnecessary\n>> commit_list_sort_by_date() call.\n> \n> Seems like an easy win. If some of the callers do not even look at the\n> result, could we avoid building it at all in those cases (e.g., by\n> passing in a NULL result pointer)?\n\nYes, at the cost of adding NULL checks to paint_down_to_common().  Which\nis probably worth it.\n> I guess there is not much to be gained, though. The result is a list of\n> merge bases, so it should usually be rather small. The benefit in your\n> patch is probably not performance, but just reducing the size of the\n> code.\nTrue.  The list can be arbitrarily long, but should only contain a\nhandful commits in normal repos.\n\nRené\n\n"},{"id":"544280","messageId":"20260529190453.GA1711766@coredump.intra.peff.net","threadId":"65701","inReplyTo":"107314f5-0057-4ed3-9bee-9dca4f424bd1@web.de","subject":"Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-05-29T19:04:53Z","receivedAt":"2026-05-29T19:04:55Z","isPatch":true,"body":"On Fri, May 29, 2026 at 05:32:58PM +0200, René Scharfe wrote:\n\n> > Seems like an easy win. If some of the callers do not even look at the\n> > result, could we avoid building it at all in those cases (e.g., by\n> > passing in a NULL result pointer)?\n> \n> Yes, at the cost of adding NULL checks to paint_down_to_common().  Which\n> is probably worth it.\n\nYeah. I thought it was only one line (where we append to \"tail\"), but\nthere are a couple spots where \"result\" is referenced directly.\n\nAnother fun fact: it looks like paint_down_to_common() makes sure to\nclean up the output list before returning an error. But the callers do\nso, too, which is redundant. That would go away if they just passed in\nNULL. :)\n\n-Peff\n"}]}