Re: [BUG] add_again() off-by-one error in custom format

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 13, 2017, 21:20 UTC
Message-ID: <xmqqwp8f4mb2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <99d19e5a-9f79-9c1e-3a23-7b2437b04ce9@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 19 quoted lines

> The difference is about the same as the one between:
>
> 	$ time git log --format="" >/dev/null
>
> 	real    0m0.463s
> 	user    0m0.448s
> 	sys     0m0.012s
>
> and:
>
> 	$ time git log --format="%h" >/dev/null
>
> 	real    0m1.062s
> 	user    0m0.636s
> 	sys     0m0.416s
>
> With caching duplicates are basically free and without it short
> hashes have to be looked up again.  Other placeholders may reduce
> the relative slowdown, depending on how expensive they are.

I think the real question is how likely people use more than one occurrence of the same thing in their custom format, and how deeply they care that --format='%h %h' costs more than --format='%h'. The cost won't of course be double (because the main traversal costs without any output), but it would be rather unreasonable to expect that --format='%h %h %h %h %h' to cost the same as --format='%h'; after all, Git is doing more for them ;-)

So in that sense, I am actually OK if we decide to remove the caching.

> Forgot a third option, probably because it's not a particularly good
> idea: Replacing the caching in pretty.c with a short static cache in
> find_unique_abbrev_r().

Indeed.