reuse cached commit buffer when parsing signatures

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>

Jeff King committed Jun 13, 2014 at 02:32 UTC 218aa3a6162b80696a82b8745daa38fa826985ae
3 files changed +12 -19
commit.c
+10 -17
@@ -1138,17 +1138,14 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)
1138 return 0;
1139 }
1140
1141 -int parse_signed_commit(const unsigned char *sha1,
1141 +int parse_signed_commit(const struct commit *commit,
1142 struct strbuf *payload, struct strbuf *signature)
1143 {
1144 +
1145 unsigned long size;
1145 - enum object_type type;
1146 - char *buffer = read_sha1_file(sha1, &type, &size);
1146 + const char *buffer = get_commit_buffer(commit, &size);
1147 int in_signature, saw_signature = -1;
1148 - char *line, *tail;
1149 -
1150 - if (!buffer || type != OBJ_COMMIT)
1151 - goto cleanup;
1148 + const char *line, *tail;
1149
1150 line = buffer;
1151 tail = buffer + size;
@@ -1156,7 +1153,7 @@ int parse_signed_commit(const unsigned char *sha1,
1153 saw_signature = 0;
1154 while (line < tail) {
1155 const char *sig = NULL;
1159 - char *next = memchr(line, '\n', tail - line);
1156 + const char *next = memchr(line, '\n', tail - line);
1157
1158 next = next ? next + 1 : tail;
1159 if (in_signature && line[0] == ' ')
@@ -1177,8 +1174,7 @@ int parse_signed_commit(const unsigned char *sha1,
1174 }
1175 line = next;
1176 }
1180 - cleanup:
1181 - free(buffer);
1177 + unuse_commit_buffer(commit, buffer);
1178 return saw_signature;
1179 }
1180
@@ -1269,8 +1265,7 @@ void check_commit_signature(const struct commit* commit, struct signature_check
1265
1266 sigc->result = 'N';
1267
1272 - if (parse_signed_commit(commit->object.sha1,
1273 - &payload, &signature) <= 0)
1268 + if (parse_signed_commit(commit, &payload, &signature) <= 0)
1269 goto out;
1270 status = verify_signed_buffer(payload.buf, payload.len,
1271 signature.buf, signature.len,
@@ -1315,11 +1310,9 @@ struct commit_extra_header *read_commit_extra_headers(struct commit *commit,
1310 {
1311 struct commit_extra_header *extra = NULL;
1312 unsigned long size;
1318 - enum object_type type;
1319 - char *buffer = read_sha1_file(commit->object.sha1, &type, &size);
1320 - if (buffer && type == OBJ_COMMIT)
1321 - extra = read_commit_extra_header_lines(buffer, size, exclude);
1322 - free(buffer);
1313 + const char *buffer = get_commit_buffer(commit, &size);
1314 + extra = read_commit_extra_header_lines(buffer, size, exclude);
1315 + unuse_commit_buffer(commit, buffer);
1316 return extra;
1317 }
1318
commit.h
+1 -1
@@ -325,7 +325,7 @@ struct merge_remote_desc {
325 */
326 struct commit *get_merge_parent(const char *name);
327
328 -extern int parse_signed_commit(const unsigned char *sha1,
328 +extern int parse_signed_commit(const struct commit *commit,
329 struct strbuf *message, struct strbuf *signature);
330 extern void print_commit_list(struct commit_list *list,
331 const char *format_cur,
log-tree.c
+1 -1
@@ -376,7 +376,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
376 struct strbuf gpg_output = STRBUF_INIT;
377 int status;
378
379 - if (parse_signed_commit(commit->object.sha1, &payload, &signature) <= 0)
379 + if (parse_signed_commit(commit, &payload, &signature) <= 0)
380 goto out;
381
382 status = verify_signed_buffer(payload.buf, payload.len,