From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Michael Giuffrida <michaelpg@chromium.org>, git@vger.kernel.org
Subject: Re: [BUG] add_again() off-by-one error in custom format
Date: Tue, 13 Jun 2017 14:20:17 -0700 [thread overview]
Message-ID: <xmqqwp8f4mb2.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <99d19e5a-9f79-9c1e-3a23-7b2437b04ce9@web.de> ("René Scharfe"'s message of "Tue, 13 Jun 2017 22:29:01 +0200")
René Scharfe <l.s.r@web.de> writes:
> 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.
next prev parent reply other threads:[~2017-06-13 21:20 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-12 3:13 [BUG] add_again() off-by-one error in custom format Michael Giuffrida
2017-06-12 22:49 ` Junio C Hamano
2017-06-13 18:09 ` René Scharfe
2017-06-13 18:29 ` Junio C Hamano
2017-06-13 20:29 ` René Scharfe
2017-06-13 21:20 ` Junio C Hamano [this message]
2017-06-14 18:24 ` René Scharfe
2017-06-15 5:56 ` Jeff King
2017-06-15 11:33 ` René Scharfe
2017-06-15 13:25 ` Jeff King
2017-06-18 10:58 ` René Scharfe
2017-06-18 11:49 ` Jeff King
2017-06-18 12:59 ` René Scharfe
2017-06-18 13:56 ` Jeff King
2017-06-22 18:19 ` René Scharfe
2017-06-22 23:15 ` Jeff King
2017-06-18 10:58 ` René Scharfe
2017-06-18 11:50 ` Jeff King
2017-06-19 4:46 ` Junio C Hamano
2017-06-22 18:19 ` [PATCH] sha1_name: cache readdir(3) results in find_short_object_filename() René Scharfe
2017-06-22 23:10 ` Jeff King
2017-06-24 12:12 ` René Scharfe
2017-06-24 12:14 ` Jeff King
2017-06-24 12:12 ` René Scharfe
2017-06-24 12:20 ` Jeff King
2017-06-24 14:09 ` René Scharfe
2017-06-24 14:12 ` Jeff King
2017-06-15 18:37 ` [BUG] add_again() off-by-one error in custom format Junio C Hamano
2017-06-13 22:24 ` SZEDER Gábor
2017-06-14 17:34 ` René Scharfe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwp8f4mb2.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
--cc=michaelpg@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).