Re: [PATCH v5 9/9] format_trailers_from_commit(): indirectly call trailer_info_get()

[Linus Arver <linusa@google.com>]

Christian Couder <christian.couder@gmail.com> writes:

> On Sat, Feb 17, 2024 at 12:09 AM Linus Arver via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Linus Arver <linusa@google.com>
>>
>> This is another preparatory refactor to unify the trailer formatters.
>>
>> Instead of calling trailer_info_get() directly, call parse_trailers()
>> which already calls trailer_info_get(). This change is a NOP because
>> format_trailer_info() only looks at the "trailers" string array, not the
>> trailer_item objects which parse_trailers() populates.
>
> Is the extra processing done by parse_trailers() compared to
> trailer_info_get() impacting performance?

I was going to answer this now but I see that you've already reached the
same conclusion as me further below. ;)

> Also when looking only at the patch, it's a bit difficult to
> understand that the "trailers" string array is the `char **trailers`
> field in `struct trailer_info` and that the trailer_item objects are
> the elements of the `struct list_head *head` linked list. It could
> also be confusing because the patch is adding a new 'trailers'
> variable with `LIST_HEAD(trailers);`. So a few more details could help
> understand what's going on.

Makes sense. Admittedly, this is one thing that did bother me at some
point but which I forgot about as I got more familiar with my own patch.

I will avoid shadowing the "trailers" word (and maybe at least add a
suffix or prefix to disambiguate it).

>> In a future patch, we'll change format_trailer_info() to use the parsed
>> trailer_item objects instead of the string array.
>
> Ok, so I guess the possible performance issue would disappear then, as
> populating the trailer_item objects will be useful.

Yep, exactly. In the larger series (some 20? 30?) commits down the road
in my local tree, I have it so that we remove `char **trailers`
entirely (because we should be using the (smarter) trailer_item objects,
not raw strings, where possible).