Christian Couder <christian.couder@xxxxxxxxx> writes: > On Sat, Feb 17, 2024 at 12:09 AM Linus Arver via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: >> >> From: Linus Arver <linusa@xxxxxxxxxx> >> >> 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).
