Skip to content

Commit

Permalink
reuse cached commit buffer when parsing signatures
Browse files Browse the repository at this point in the history
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, attached to the "struct commit". 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 the 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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Jun 13, 2014
1 parent 8597ea3 commit 218aa3a
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 19 deletions.
27 changes: 10 additions & 17 deletions commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -1138,25 +1138,22 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)
return 0;
}

int parse_signed_commit(const unsigned char *sha1,
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 = get_commit_buffer(commit, &size);
int in_signature, saw_signature = -1;
char *line, *tail;

if (!buffer || type != OBJ_COMMIT)
goto cleanup;
const char *line, *tail;

line = buffer;
tail = buffer + size;
in_signature = 0;
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] == ' ')
Expand All @@ -1177,8 +1174,7 @@ int parse_signed_commit(const unsigned char *sha1,
}
line = next;
}
cleanup:
free(buffer);
unuse_commit_buffer(commit, buffer);
return saw_signature;
}

Expand Down Expand Up @@ -1269,8 +1265,7 @@ void check_commit_signature(const struct commit* commit, struct signature_check

sigc->result = 'N';

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,
signature.buf, signature.len,
Expand Down Expand Up @@ -1315,11 +1310,9 @@ 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);
if (buffer && type == OBJ_COMMIT)
extra = read_commit_extra_header_lines(buffer, size, exclude);
free(buffer);
const char *buffer = get_commit_buffer(commit, &size);
extra = read_commit_extra_header_lines(buffer, size, exclude);
unuse_commit_buffer(commit, buffer);
return extra;
}

Expand Down
2 changes: 1 addition & 1 deletion commit.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,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,
Expand Down
2 changes: 1 addition & 1 deletion log-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 218aa3a

Please sign in to comment.