Subject: Re: [PATCH 1/4] line-log: avoid unnecessary tree diffs when processing merge commits

From: SZEDER Gábor <[email-masked]>
Date: Thu, 28 Aug 2025 22:27:01 +0200

On Mon, Aug 25, 2025 at 08:35:53AM -0700, Junio C Hamano wrote:
> SZEDER Gábor <[email-masked]> writes:
> 
> > @@ -1209,7 +1202,6 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c
> >  static int process_ranges_merge_commit(struct rev_info *rev, struct commit *commit,
> >  				       struct line_log_data *range)
> >  {
> > -	struct diff_queue_struct *diffqueues;
> >  	struct line_log_data **cand;
> >  	struct commit **parents;
> >  	struct commit_list *p;
> > @@ -1220,20 +1212,19 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
> >  	if (nparents > 1 && rev->first_parent_only)
> >  		nparents = 1;
> >  
> > -	ALLOC_ARRAY(diffqueues, nparents);
> >  	CALLOC_ARRAY(cand, nparents);
> >  	ALLOC_ARRAY(parents, nparents);
> >  
> >  	p = commit->parents;
> >  	for (i = 0; i < nparents; i++) {
> > +		struct diff_queue_struct diffqueue = DIFF_QUEUE_INIT;
> > +		int changed;
> >  		parents[i] = p->item;
> >  		p = p->next;
> > -		queue_diffs(range, &rev->diffopt, &diffqueues[i], commit, parents[i]);
> > -	}
> > +		queue_diffs(range, &rev->diffopt, &diffqueue, commit, parents[i]);
> >  
> > -	for (i = 0; i < nparents; i++) {
> > -		int changed;
> > -		changed = process_all_files(&cand[i], rev, &diffqueues[i], range);
> > +		changed = process_all_files(&cand[i], rev, &diffqueue, range);
> > +		diff_queue_clear(&diffqueue);
> >  		if (!changed) {
> >  			/*
> >  			 * This parent can take all the blame, so we
> 
> This is surprisingly small change that eliminates quite a lot of
> waste.  Nicely done.

It's funny you say that...

This patch series just turned 6 years old this weekend, and up until
Sunday morning this first patch was actually two, because the
optimization and the removal of the now unnecessary diffqueues array
were two separate patches that I finally decided to squash together.

Here is the diff of that optimization-only patch :)

  ---- >8 ----

 line-log.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/line-log.c b/line-log.c
index 07f2154e84..b3766c67ea 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1220,19 +1220,17 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
 	if (nparents > 1 && rev->first_parent_only)
 		nparents = 1;
 
-	ALLOC_ARRAY(diffqueues, nparents);
+	CALLOC_ARRAY(diffqueues, nparents);
 	CALLOC_ARRAY(cand, nparents);
 	ALLOC_ARRAY(parents, nparents);
 
 	p = commit->parents;
 	for (i = 0; i < nparents; i++) {
+		int changed;
 		parents[i] = p->item;
 		p = p->next;
 		queue_diffs(range, &rev->diffopt, &diffqueues[i], commit, parents[i]);
-	}
 
-	for (i = 0; i < nparents; i++) {
-		int changed;
 		changed = process_all_files(&cand[i], rev, &diffqueues[i], range);
 		if (!changed) {
 			/*