Thread (8 messages) 8 messages, 3 authors, 2021-02-17

Re: [PATCH 1/1] blame: remove unnecessary use of get_commit_info()

flat view

From: Rafael Silva <hidden>
Date: 2021-02-16 22:27:04

Taylor Blau [off-list ref] writes:
On Tue, Feb 16, 2021 at 05:31:51PM +0100, Rafael Silva wrote:
quoted
When `git blame --color-by-age`, the determine_line_heat() is called to
select how to color the output based on the commit author's date.  It
uses the get_commit_info() to parse the information into a `commit_info`
structure, however, this is actually unnecessary because the
determine_line_heat() caller also does the same.
Interesting. It looks like this micro-optimization could have been
safely performed as early as 25d5f52901 (builtin/blame: highlight
recently changed lines, 2018-04-23), which is when this feature was
originally introduced.

I looked at 25d5f52901 to see if there was any reason that we didn't at
the time, but couldn't find anything.

So this looks correct to me. I'm a little disappointed that some of
your more detailed performance numbers from the cover letter didn't make
it into the patch description, but it may not be worth belaboring the
point further.

    Reviewed-by: Taylor Blau [off-list ref]
Thanks for reviewing this patch.

I wasn't sure whether adding the performance number into the patch was a
good idea or not.  After reading your message and the response from
Derrick and Junio, I'll definitely re-roll this patch adding those
information.
Thanks,
Taylor
-- 
Thanks
Rafael
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help