Am 13.06.2017 um 20:29 schrieb Junio C Hamano:
René Scharfe <l.s.r@xxxxxx> writes:
Indeed, a very nice bug report!
I think the call to format_commit_one() needs to be taught to pass
'magic' through, and then add_again() mechanism needs to be told not
to cache when magic is in effect, or something like that.
Perhaps something along this line...?
pretty.c | 64 ++++++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 38 insertions(+), 26 deletions(-)
That looks quite big. You even invent a way to classify magic.
Hmph, by "a way to classify", do you mean the enum? That thing was
there from before, just moved.
Oh, yes, sorry. I didn't even get that far into the patch. (I'll
better go to bed after hitting send..)
Also I think we do not have to change add_again() at all, because
chunk->off tolerates being given a garbage value as long as
chunk->len stays 0, and add_again() does not change chunk->len at
all.
Which cuts the diffstat down to
pretty.c | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
Does the caching feature justify the added complexity?
That's a good question. I thought about your second alternative
while adding the "don't cache" thing, but if we can measure and find
out that we are not gaining much from caching, certainly that sounds
much simpler.
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.
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().
René