[PATCH] blame: remove parameter detailed in get_commit_info()

[PATCH] blame: remove parameter detailed in get_commit_info()

[Han Young]

The get_commit_info() function accepts a parameter that can be used to
stop the commit parsing early.
However, none of the callers use this feature, and testing proved that
the performance gain of stopping parsing early is negligible.

Signed-off-by: Han Young <hanyang.tony@bytedance.com>
---
 builtin/blame.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 91586e685..dc934abef 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,9 +197,7 @@ static void commit_info_destroy(struct commit_info *ci)
 	strbuf_release(&ci->summary);
 }
 
-static void get_commit_info(struct commit *commit,
-			    struct commit_info *ret,
-			    int detailed)
+static void get_commit_info(struct commit *commit, struct commit_info *ret)
 {
 	int len;
 	const char *subject, *encoding;
@@ -211,11 +209,6 @@ static void get_commit_info(struct commit *commit,
 		    &ret->author, &ret->author_mail,
 		    &ret->author_time, &ret->author_tz);
 
-	if (!detailed) {
-		repo_unuse_commit_buffer(the_repository, commit, message);
-		return;
-	}
-
 	get_ac_line(message, "\ncommitter ",
 		    &ret->committer, &ret->committer_mail,
 		    &ret->committer_time, &ret->committer_tz);
@@ -263,7 +256,7 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 		return 0;
 
 	suspect->commit->object.flags |= METAINFO_SHOWN;
-	get_commit_info(suspect->commit, &ci, 1);
+	get_commit_info(suspect->commit, &ci);
 	printf("author %s\n", ci.author.buf);
 	printf("author-mail %s\n", ci.author_mail.buf);
 	printf("author-time %"PRItime"\n", ci.author_time);
@@ -471,7 +464,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 	const char *default_color = NULL, *color = NULL, *reset = NULL;
 
-	get_commit_info(suspect->commit, &ci, 1);
+	get_commit_info(suspect->commit, &ci);
 	oid_to_hex_r(hex, &suspect->commit->object.oid);
 
 	cp = blame_nth_line(sb, ent->lno);
@@ -665,7 +658,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
 			struct commit_info ci = COMMIT_INFO_INIT;
 			suspect->commit->object.flags |= METAINFO_SHOWN;
-			get_commit_info(suspect->commit, &ci, 1);
+			get_commit_info(suspect->commit, &ci);
 			if (*option & OUTPUT_SHOW_EMAIL)
 				num = utf8_strwidth(ci.author_mail.buf);
 			else
-- 
2.50.0

Re: [PATCH] blame: remove parameter detailed in get_commit_info()

[Patrick Steinhardt]

On Mon, Jul 28, 2025 at 11:55:48AM +0800, Han Young wrote:
> The get_commit_info() function accepts a parameter that can be used to
> stop the commit parsing early.
> However, none of the callers use this feature, and testing proved that
> the performance gain of stopping parsing early is negligible.

Funny enough it doesn't seem like the `detailed` field was ever used.
`get_commit_info()` was introduced all the way back in cee7f245dca
(git-pickaxe: blame rewritten., 2006-10-19), and even back then all
callers passed `1` as the `detailed` parameter.

So this patch looks obviously correct to me, thanks!

Patrick

Re: [PATCH] blame: remove parameter detailed in get_commit_info()

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Jul 28, 2025 at 11:55:48AM +0800, Han Young wrote:
>> The get_commit_info() function accepts a parameter that can be used to
>> stop the commit parsing early.
>> However, none of the callers use this feature, and testing proved that
>> the performance gain of stopping parsing early is negligible.

Is it negligible but measurable, or negligible and unmeasurable?

> Funny enough it doesn't seem like the `detailed` field was ever used.
> `get_commit_info()` was introduced all the way back in cee7f245dca
> (git-pickaxe: blame rewritten., 2006-10-19), and even back then all
> callers passed `1` as the `detailed` parameter.
>
> So this patch looks obviously correct to me, thanks!

I am all for simplifying.  It is great to see us lose more lines.

Thanks.

Re: [External] Re: [PATCH] blame: remove parameter detailed in get_commit_info()

[Han Young]

On Mon, Jul 28, 2025 at 11:40 PM Junio C Hamano <gitster@pobox.com> wrote:
> Is it negligible but measurable, or negligible and unmeasurable?
On a 5000-line file with a fairly long history, running
"git blame --porcelain FILE" for 100 times, the speedup is less
than 1 second. Considering the total run time is 180 seconds,
I think it could be system noise. So negligible and unmeasurable.

Thanks.

Re: [External] Re: [PATCH] blame: remove parameter detailed in get_commit_info()

[Junio C Hamano]

Han Young <hanyang.tony@bytedance.com> writes:

> On Mon, Jul 28, 2025 at 11:40 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Is it negligible but measurable, or negligible and unmeasurable?
> On a 5000-line file with a fairly long history, running
> "git blame --porcelain FILE" for 100 times, the speedup is less
> than 1 second. Considering the total run time is 180 seconds,
> I think it could be system noise. So negligible and unmeasurable.

OK.  Sounds good.