Re: [External] Re: [PATCH] blame: only parse detailed commit info if needed

From: Han Young <hanyang.tony@bytedance.com>
Date: Jul 23, 2025, 06:02 UTC
Message-ID: <CAG1j3zHn_wp-R6tOJqCFi+wKro_t0oNj+aHsnbLw+O66i2YvQQ@mail.gmail.com>
In-Reply-To: <aH-dGhHm2xuY4yT3@pks.im>

On Tue, Jul 22, 2025 at 10:16 PM Patrick Steinhardt <ps@pks.im> wrote:

>   - What does this buy us? I guess the answer is performance, but it
>     would be sure to quantify in which scenarios and how much of a
>     speedup this buys us.

I actually ran some performance tests before submitting this patch, 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 the performance gain is negligible (the speed increase could even be due to system noise).

> - Any reasoning why those two callers don't need the information.
> Reviewers can try to piece it together manually, but it would be
> nice to hold their hand and lead them through the change.

These two callers only access the author information part of the commit_info struct, before discarding the commit_info object. Sorry for not including this information in the patch description.

On Wed, Jul 23, 2025 at 1:08 AM Junio C Hamano <gitster@pobox.com> wrote:

> If the answer to the first question is "well, not really?", then
> another thing to consider would be if we want to remove that
> short-cut as conditionally grabbing only just some pieces of
> information without getting others is not helping.

All callers of the get_commit_info function request detailed commit information. Removing the shortcut would reduce the complexity of the codebase. I will send another patch to remove the "detailed" param from the get_commit_info.

Thanks