diff options
author | Jeff King <peff@peff.net> | 2014-05-30 14:34:41 -0400 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2014-05-30 11:55:07 -0700 |
commit | 333e9b661f6a64a9b552230c0dc987d9e69bceae (patch) | |
tree | 5a503c701d224431d431c5413db8948092aeec39 | |
parent | 7bbc4e8fdb33e0a8e42e77cc05460d4c4f615f4d (diff) | |
download | git-jk/show-sig-reuse-commit-buffer.tar.gz |
reuse commit->buffer when parsing signaturesjk/show-sig-reuse-commit-buffer
When we call show_signature or show_mergetag, we read the
commit object fresh via read_sha1_file and reparse its
headers. However, in most cases we already have the object
data available as commit->buffer. This is partially
laziness in dealing with the memory allocation issues, but
partially defensive programming, in that we would always
want to verify a clean version of the buffer (not one that
might have been munged by other users of the commit).
However, we do not currently ever munge commit->buffer, and
not using the already-available buffer carries a fairly big
performance penalty when we are looking at a large number of
commits. Here are timings on linux.git:
[baseline, no signatures]
$ time git log >/dev/null
real 0m4.902s
user 0m4.784s
sys 0m0.120s
[before]
$ time git log --show-signature >/dev/null
real 0m14.735s
user 0m9.964s
sys 0m0.944s
[after]
$ time git log --show-signature >/dev/null
real 0m9.981s
user 0m5.260s
sys 0m0.936s
Note that our user CPU time drops almost in half, close to
the non-signature case, but we do still spend more
wall-clock and system time, presumably from dealing with
gpg.
An alternative to this is to note that most commits do not
have signatures (less than 1% in this repo), yet we pay the
re-parsing cost for every commit just to find out if it has
a mergetag or signature. If we checked that when parsing the
commit initially, we could avoid re-examining most commits
later on. Even if we did pursue that direction, however,
this would still speed up the cases where we _do_ have
signatures. So it's probably worth doing either way.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | commit.c | 44 | ||||
-rw-r--r-- | commit.h | 2 | ||||
-rw-r--r-- | log-tree.c | 2 |
3 files changed, 38 insertions, 10 deletions
@@ -1083,14 +1083,42 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid) return 0; } -int parse_signed_commit(const unsigned char *sha1, +/* + * Return the contents of the object pointed to by commit, as + * if read by read_sha1_file. However, in cases where the commit's + * data is already in memory, return that as an optimization. + * + * The resulting buffer may or may not be freshly allocated, + * and should only be freed by free_commit_buffer. + */ +static const char *read_commit_buffer(const struct commit *commit, + enum object_type *type, + unsigned long *size) +{ + if (commit->buffer) { + *type = OBJ_COMMIT; + *size = strlen(commit->buffer); + return commit->buffer; + } + + return read_sha1_file(commit->object.sha1, type, size); +} + +static void free_commit_buffer(const char *buffer, const struct commit *commit) +{ + if (buffer != commit->buffer) + free((char *)buffer); +} + +int parse_signed_commit(const struct commit *commit, struct strbuf *payload, struct strbuf *signature) { + unsigned long size; enum object_type type; - char *buffer = read_sha1_file(sha1, &type, &size); + const char *buffer = read_commit_buffer(commit, &type, &size); int in_signature, saw_signature = -1; - char *line, *tail; + const char *line, *tail; if (!buffer || type != OBJ_COMMIT) goto cleanup; @@ -1101,7 +1129,7 @@ int parse_signed_commit(const unsigned char *sha1, saw_signature = 0; while (line < tail) { const char *sig = NULL; - char *next = memchr(line, '\n', tail - line); + const char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; if (in_signature && line[0] == ' ') @@ -1123,7 +1151,7 @@ int parse_signed_commit(const unsigned char *sha1, line = next; } cleanup: - free(buffer); + free_commit_buffer(buffer, commit); return saw_signature; } @@ -1216,7 +1244,7 @@ void check_commit_signature(const struct commit* commit, struct signature_check sigc->result = 'N'; - if (parse_signed_commit(commit->object.sha1, + if (parse_signed_commit(commit, &payload, &signature) <= 0) goto out; status = verify_signed_buffer(payload.buf, payload.len, @@ -1263,10 +1291,10 @@ struct commit_extra_header *read_commit_extra_headers(struct commit *commit, struct commit_extra_header *extra = NULL; unsigned long size; enum object_type type; - char *buffer = read_sha1_file(commit->object.sha1, &type, &size); + const char *buffer = read_commit_buffer(commit, &type, &size); if (buffer && type == OBJ_COMMIT) extra = read_commit_extra_header_lines(buffer, size, exclude); - free(buffer); + free_commit_buffer(buffer, commit); return extra; } @@ -257,7 +257,7 @@ struct merge_remote_desc { */ struct commit *get_merge_parent(const char *name); -extern int parse_signed_commit(const unsigned char *sha1, +extern int parse_signed_commit(const struct commit *commit, struct strbuf *message, struct strbuf *signature); extern void print_commit_list(struct commit_list *list, const char *format_cur, diff --git a/log-tree.c b/log-tree.c index 1982631ca4..2affe8150d 100644 --- a/log-tree.c +++ b/log-tree.c @@ -376,7 +376,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit) struct strbuf gpg_output = STRBUF_INIT; int status; - if (parse_signed_commit(commit->object.sha1, &payload, &signature) <= 0) + if (parse_signed_commit(commit, &payload, &signature) <= 0) goto out; status = verify_signed_buffer(payload.buf, payload.len, |