# [PATCH] blame: remove parameter detailed in get_commit_info() 5 messages from 2025-07-28 to 2025-07-29. Participants: Han Young, Patrick Steinhardt, Junio C Hamano. Thread: https://gitlist.dev/t/63864 ## Han Young, 2025-07-28 03:55 Subject: [PATCH] blame: remove parameter detailed in get_commit_info() Message-ID: <20250728035548.94277-1-hanyang.tony@bytedance.com> URL: https://gitlist.dev/e/20250728035548.94277-1-hanyang.tony%40bytedance.com ``` 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 --- 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 ``` ## Patrick Steinhardt, 2025-07-28 06:02 Subject: Re: [PATCH] blame: remove parameter detailed in get_commit_info() Message-ID: URL: https://gitlist.dev/e/aIcSYs7LxkJeRA-9%40pks.im In-Reply-To: <20250728035548.94277-1-hanyang.tony@bytedance.com> ``` 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 ``` ## Junio C Hamano, 2025-07-28 15:40 Subject: Re: [PATCH] blame: remove parameter detailed in get_commit_info() Message-ID: URL: https://gitlist.dev/e/xmqq4iuwxr12.fsf%40gitster.g In-Reply-To: ``` Patrick Steinhardt 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. ``` ## Han Young, 2025-07-29 02:50 Subject: Re: [External] Re: [PATCH] blame: remove parameter detailed in get_commit_info() Message-ID: URL: https://gitlist.dev/e/CAG1j3zHPU_moH51O4i97c7ofuGWiRKunZmtZe2OUAKqAXAKg0g%40mail.gmail.com In-Reply-To: ``` On Mon, Jul 28, 2025 at 11:40 PM Junio C Hamano 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. ``` ## Junio C Hamano, 2025-07-29 05:02 Subject: Re: [External] Re: [PATCH] blame: remove parameter detailed in get_commit_info() Message-ID: URL: https://gitlist.dev/e/xmqq1ppzsi7l.fsf%40gitster.g In-Reply-To: ``` Han Young writes: > On Mon, Jul 28, 2025 at 11:40 PM Junio C Hamano 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. ```