{"thread":{"id":"63864","subject":"[PATCH] blame: remove parameter detailed in get_commit_info()","startedAt":"2025-07-28T03:55:56Z","lastAt":"2025-07-29T05:02:09Z","messageCount":5,"participants":["Han Young","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"522841","messageId":"20250728035548.94277-1-hanyang.tony@bytedance.com","threadId":"63864","inReplyTo":null,"subject":"[PATCH] blame: remove parameter detailed in get_commit_info()","fromName":"Han Young","fromEmail":"hanyang.tony@bytedance.com","sentAt":"2025-07-28T03:55:48Z","receivedAt":"2025-07-28T03:55:56Z","isPatch":true,"sender":{"key":"hanyang.tony@bytedance.com","avatar":"https://avatars.githubusercontent.com/u/108711387?v=4"},"body":"The get_commit_info() function accepts a parameter that can be used to\nstop the commit parsing early.\nHowever, none of the callers use this feature, and testing proved that\nthe performance gain of stopping parsing early is negligible.\n\nSigned-off-by: Han Young \n---\n builtin/blame.c | 15 ++++-----------\n 1 file changed, 4 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 91586e685..dc934abef 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -197,9 +197,7 @@ static void commit_info_destroy(struct commit_info *ci)\n \tstrbuf_release(&ci->summary);\n }\n \n-static void get_commit_info(struct commit *commit,\n-\t\t\t struct commit_info *ret,\n-\t\t\t int detailed)\n+static void get_commit_info(struct commit *commit, struct commit_info *ret)\n {\n \tint len;\n \tconst char *subject, *encoding;\n@@ -211,11 +209,6 @@ static void get_commit_info(struct commit *commit,\n \t\t &ret->author, &ret->author_mail,\n \t\t &ret->author_time, &ret->author_tz);\n \n-\tif (!detailed) {\n-\t\trepo_unuse_commit_buffer(the_repository, commit, message);\n-\t\treturn;\n-\t}\n-\n \tget_ac_line(message, \"\\ncommitter \",\n \t\t &ret->committer, &ret->committer_mail,\n \t\t &ret->committer_time, &ret->committer_tz);\n@@ -263,7 +256,7 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)\n \t\treturn 0;\n \n \tsuspect->commit->object.flags |= METAINFO_SHOWN;\n-\tget_commit_info(suspect->commit, &ci, 1);\n+\tget_commit_info(suspect->commit, &ci);\n \tprintf(\"author %s\\n\", ci.author.buf);\n \tprintf(\"author-mail %s\\n\", ci.author_mail.buf);\n \tprintf(\"author-time %\"PRItime\"\\n\", ci.author_time);\n@@ -471,7 +464,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int\n \tint show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);\n \tconst char *default_color = NULL, *color = NULL, *reset = NULL;\n \n-\tget_commit_info(suspect->commit, &ci, 1);\n+\tget_commit_info(suspect->commit, &ci);\n \toid_to_hex_r(hex, &suspect->commit->object.oid);\n \n \tcp = blame_nth_line(sb, ent->lno);\n@@ -665,7 +658,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)\n \t\tif (!(suspect->commit->object.flags & METAINFO_SHOWN)) {\n \t\t\tstruct commit_info ci = COMMIT_INFO_INIT;\n \t\t\tsuspect->commit->object.flags |= METAINFO_SHOWN;\n-\t\t\tget_commit_info(suspect->commit, &ci, 1);\n+\t\t\tget_commit_info(suspect->commit, &ci);\n \t\t\tif (*option & OUTPUT_SHOW_EMAIL)\n \t\t\t\tnum = utf8_strwidth(ci.author_mail.buf);\n \t\t\telse\n-- \n2.50.0\n\n"},{"id":"522842","messageId":"aIcSYs7LxkJeRA-9@pks.im","threadId":"63864","inReplyTo":"20250728035548.94277-1-hanyang.tony@bytedance.com","subject":"Re: [PATCH] blame: remove parameter detailed in get_commit_info()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-07-28T06:02:10Z","receivedAt":"2025-07-28T06:02:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jul 28, 2025 at 11:55:48AM +0800, Han Young wrote:\n> The get_commit_info() function accepts a parameter that can be used to\n> stop the commit parsing early.\n> However, none of the callers use this feature, and testing proved that\n> the performance gain of stopping parsing early is negligible.\n\nFunny enough it doesn't seem like the `detailed` field was ever used.\n`get_commit_info()` was introduced all the way back in cee7f245dca\n(git-pickaxe: blame rewritten., 2006-10-19), and even back then all\ncallers passed `1` as the `detailed` parameter.\n\nSo this patch looks obviously correct to me, thanks!\n\nPatrick\n"},{"id":"522860","messageId":"xmqq4iuwxr12.fsf@gitster.g","threadId":"63864","inReplyTo":"aIcSYs7LxkJeRA-9@pks.im","subject":"Re: [PATCH] blame: remove parameter detailed in get_commit_info()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-28T15:40:25Z","receivedAt":"2025-07-28T15:40:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Mon, Jul 28, 2025 at 11:55:48AM +0800, Han Young wrote:\n>> The get_commit_info() function accepts a parameter that can be used to\n>> stop the commit parsing early.\n>> However, none of the callers use this feature, and testing proved that\n>> the performance gain of stopping parsing early is negligible.\n\nIs it negligible but measurable, or negligible and unmeasurable?\n\n> Funny enough it doesn't seem like the `detailed` field was ever used.\n> `get_commit_info()` was introduced all the way back in cee7f245dca\n> (git-pickaxe: blame rewritten., 2006-10-19), and even back then all\n> callers passed `1` as the `detailed` parameter.\n>\n> So this patch looks obviously correct to me, thanks!\n\nI am all for simplifying. It is great to see us lose more lines.\n\nThanks.\n"},{"id":"522913","messageId":"CAG1j3zHPU_moH51O4i97c7ofuGWiRKunZmtZe2OUAKqAXAKg0g@mail.gmail.com","threadId":"63864","inReplyTo":"xmqq4iuwxr12.fsf@gitster.g","subject":"Re: [External] Re: [PATCH] blame: remove parameter detailed in get_commit_info()","fromName":"Han Young","fromEmail":"hanyang.tony@bytedance.com","sentAt":"2025-07-29T02:50:16Z","receivedAt":"2025-07-29T02:50:27Z","isPatch":true,"sender":{"key":"hanyang.tony@bytedance.com","avatar":"https://avatars.githubusercontent.com/u/108711387?v=4"},"body":"On Mon, Jul 28, 2025 at 11:40 PM Junio C Hamano wrote:\n> Is it negligible but measurable, or negligible and unmeasurable?\nOn a 5000-line file with a fairly long history, running\n\"git blame --porcelain FILE\" for 100 times, the speedup is less\nthan 1 second. Considering the total run time is 180 seconds,\nI think it could be system noise. So negligible and unmeasurable.\n\nThanks.\n"},{"id":"522914","messageId":"xmqq1ppzsi7l.fsf@gitster.g","threadId":"63864","inReplyTo":"CAG1j3zHPU_moH51O4i97c7ofuGWiRKunZmtZe2OUAKqAXAKg0g@mail.gmail.com","subject":"Re: [External] Re: [PATCH] blame: remove parameter detailed in get_commit_info()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-29T05:02:06Z","receivedAt":"2025-07-29T05:02:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han Young writes:\n\n> On Mon, Jul 28, 2025 at 11:40 PM Junio C Hamano wrote:\n>> Is it negligible but measurable, or negligible and unmeasurable?\n> On a 5000-line file with a fairly long history, running\n> \"git blame --porcelain FILE\" for 100 times, the speedup is less\n> than 1 second. Considering the total run time is 180 seconds,\n> I think it could be system noise. So negligible and unmeasurable.\n\nOK. Sounds good.\n"}]}