Skip to content

Commit 8061ae8

Browse files
gitstergitster
committed
Merge branch 'jk/commit-buffer-length'
Move "commit->buffer" out of the in-core commit object and keep track of their lengths. Use this to optimize the code paths to validate GPG signatures in commit objects. * jk/commit-buffer-length: reuse cached commit buffer when parsing signatures commit: record buffer length in cache commit: convert commit->buffer to a slab commit-slab: provide a static initializer use get_commit_buffer everywhere convert logmsg_reencode to get_commit_buffer use get_commit_buffer to avoid duplicate code use get_cached_commit_buffer where appropriate provide helpers to access the commit buffer provide a helper to set the commit buffer provide a helper to free commit buffer sequencer: use logmsg_reencode in get_message logmsg_reencode: return const buffer do not create "struct commit" with xcalloc commit: push commit_index update into alloc_commit_node alloc: include any-object allocations in alloc_report replace dangerous uses of strbuf_attach commit_tree: take a pointer/len pair rather than a const strbuf
2 parents 95acfc2 + 218aa3a commit 8061ae8

27 files changed

+284
-197
lines changed

‎alloc.c‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,32 @@ union any_object {
4747

4848
DEFINE_ALLOCATOR(blob, struct blob)
4949
DEFINE_ALLOCATOR(tree, struct tree)
50-
DEFINE_ALLOCATOR(commit, struct commit)
50+
DEFINE_ALLOCATOR(raw_commit, struct commit)
5151
DEFINE_ALLOCATOR(tag, struct tag)
5252
DEFINE_ALLOCATOR(object, union any_object)
5353

54+
void *alloc_commit_node(void)
55+
{
56+
static int commit_count;
57+
struct commit *c = alloc_raw_commit_node();
58+
c->index = commit_count++;
59+
return c;
60+
}
61+
5462
static void report(const char *name, unsigned int count, size_t size)
5563
{
5664
fprintf(stderr, "%10s: %8u (%"PRIuMAX" kB)\n",
5765
name, count, (uintmax_t) size);
5866
}
5967

60-
#define REPORT(name) \
61-
report(#name, name##_allocs, name##_allocs * sizeof(struct name) >> 10)
68+
#define REPORT(name, type) \
69+
report(#name, name##_allocs, name##_allocs * sizeof(type) >> 10)
6270

6371
void alloc_report(void)
6472
{
65-
REPORT(blob);
66-
REPORT(tree);
67-
REPORT(commit);
68-
REPORT(tag);
73+
REPORT(blob, struct blob);
74+
REPORT(tree, struct tree);
75+
REPORT(raw_commit, struct commit);
76+
REPORT(tag, struct tag);
77+
REPORT(object, union any_object);
6978
}

‎builtin/blame.c‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,7 +1655,7 @@ static void get_commit_info(struct commit *commit,
16551655
{
16561656
int len;
16571657
const char *subject, *encoding;
1658-
char *message;
1658+
const char *message;
16591659

16601660
commit_info_init(ret);
16611661

@@ -1666,7 +1666,7 @@ static void get_commit_info(struct commit *commit,
16661666
&ret->author_time, &ret->author_tz);
16671667

16681668
if (!detailed) {
1669-
logmsg_free(message, commit);
1669+
unuse_commit_buffer(commit, message);
16701670
return;
16711671
}
16721672

@@ -1680,7 +1680,7 @@ static void get_commit_info(struct commit *commit,
16801680
else
16811681
strbuf_addf(&ret->summary, "(%s)", sha1_to_hex(commit->object.sha1));
16821682

1683-
logmsg_free(message, commit);
1683+
unuse_commit_buffer(commit, message);
16841684
}
16851685

16861686
/*
@@ -2251,6 +2251,18 @@ static void append_merge_parents(struct commit_list **tail)
22512251
strbuf_release(&line);
22522252
}
22532253

2254+
/*
2255+
* This isn't as simple as passing sb->buf and sb->len, because we
2256+
* want to transfer ownership of the buffer to the commit (so we
2257+
* must use detach).
2258+
*/
2259+
static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)
2260+
{
2261+
size_t len;
2262+
void *buf = strbuf_detach(sb, &len);
2263+
set_commit_buffer(c, buf, len);
2264+
}
2265+
22542266
/*
22552267
* Prepare a dummy commit that represents the work tree (or staged) item.
22562268
* Note that annotating work tree item never works in the reverse.
@@ -2272,7 +2284,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
22722284
struct strbuf msg = STRBUF_INIT;
22732285

22742286
time(&now);
2275-
commit = xcalloc(1, sizeof(*commit));
2287+
commit = alloc_commit_node();
22762288
commit->object.parsed = 1;
22772289
commit->date = now;
22782290
commit->object.type = OBJ_COMMIT;
@@ -2299,7 +2311,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
22992311
ident, ident, path,
23002312
(!contents_from ? path :
23012313
(!strcmp(contents_from, "-") ? "standard input" : contents_from)));
2302-
commit->buffer = strbuf_detach(&msg, NULL);
2314+
set_commit_buffer_from_strbuf(commit, &msg);
23032315

23042316
if (!contents_from || strcmp("-", contents_from)) {
23052317
struct stat st;

‎builtin/commit-tree.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
123123
die_errno("git commit-tree: failed to read");
124124
}
125125

126-
if (commit_tree(&buffer, tree_sha1, parents, commit_sha1,
127-
NULL, sign_commit)) {
126+
if (commit_tree(buffer.buf, buffer.len, tree_sha1, parents,
127+
commit_sha1, NULL, sign_commit)) {
128128
strbuf_release(&buffer);
129129
return 1;
130130
}

‎builtin/commit.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1745,8 +1745,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
17451745
append_merge_tag_headers(parents, &tail);
17461746
}
17471747

1748-
if (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1,
1749-
author_ident.buf, sign_commit, extra)) {
1748+
if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
1749+
parents, sha1, author_ident.buf, sign_commit, extra)) {
17501750
rollback_index_files();
17511751
die(_("failed to write commit object"));
17521752
}

‎builtin/fast-export.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ static const char *find_encoding(const char *begin, const char *end)
282282
static void handle_commit(struct commit *commit, struct rev_info *rev)
283283
{
284284
int saved_output_format = rev->diffopt.output_format;
285+
const char *commit_buffer;
285286
const char *author, *author_end, *committer, *committer_end;
286287
const char *encoding, *message;
287288
char *reencoded = NULL;
@@ -291,7 +292,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
291292
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
292293

293294
parse_commit_or_die(commit);
294-
author = strstr(commit->buffer, "\nauthor ");
295+
commit_buffer = get_commit_buffer(commit, NULL);
296+
author = strstr(commit_buffer, "\nauthor ");
295297
if (!author)
296298
die ("Could not find author in commit %s",
297299
sha1_to_hex(commit->object.sha1));
@@ -338,6 +340,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
338340
? strlen(message) : 0),
339341
reencoded ? reencoded : message ? message : "");
340342
free(reencoded);
343+
unuse_commit_buffer(commit, commit_buffer);
341344

342345
for (i = 0, p = commit->parents; p; p = p->next) {
343346
int mark = get_object_mark(&p->item->object);

‎builtin/fmt-merge-msg.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,14 @@ static void add_branch_desc(struct strbuf *out, const char *name)
230230
static void record_person(int which, struct string_list *people,
231231
struct commit *commit)
232232
{
233+
const char *buffer;
233234
char *name_buf, *name, *name_end;
234235
struct string_list_item *elem;
235236
const char *field;
236237

237238
field = (which == 'a') ? "\nauthor " : "\ncommitter ";
238-
name = strstr(commit->buffer, field);
239+
buffer = get_commit_buffer(commit, NULL);
240+
name = strstr(buffer, field);
239241
if (!name)
240242
return;
241243
name += strlen(field);
@@ -247,6 +249,7 @@ static void record_person(int which, struct string_list *people,
247249
if (name_end < name)
248250
return;
249251
name_buf = xmemdupz(name, name_end - name + 1);
252+
unuse_commit_buffer(commit, buffer);
250253

251254
elem = string_list_lookup(people, name_buf);
252255
if (!elem) {

‎builtin/fsck.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,7 @@ static int fsck_obj(struct object *obj)
310310
if (obj->type == OBJ_COMMIT) {
311311
struct commit *commit = (struct commit *) obj;
312312

313-
free(commit->buffer);
314-
commit->buffer = NULL;
313+
free_commit_buffer(commit);
315314

316315
if (!commit->parents && show_root)
317316
printf("root %s\n", sha1_to_hex(commit->object.sha1));

‎builtin/index-pack.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,8 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
786786
}
787787
if (obj->type == OBJ_COMMIT) {
788788
struct commit *commit = (struct commit *) obj;
789-
commit->buffer = NULL;
789+
if (detach_commit_buffer(commit, NULL) != data)
790+
die("BUG: parse_object_buffer transmogrified our buffer");
790791
}
791792
obj->flags |= FLAG_CHECKED;
792793
}

‎builtin/log.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,7 @@ static int cmd_log_walk(struct rev_info *rev)
347347
rev->max_count++;
348348
if (!rev->reflog_info) {
349349
/* we allow cycles in reflog ancestry */
350-
free(commit->buffer);
351-
commit->buffer = NULL;
350+
free_commit_buffer(commit);
352351
}
353352
free_commit_list(commit->parents);
354353
commit->parents = NULL;
@@ -925,9 +924,12 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
925924
log_write_email_headers(rev, head, &pp.subject, &pp.after_subject,
926925
&need_8bit_cte);
927926

928-
for (i = 0; !need_8bit_cte && i < nr; i++)
929-
if (has_non_ascii(list[i]->buffer))
927+
for (i = 0; !need_8bit_cte && i < nr; i++) {
928+
const char *buf = get_commit_buffer(list[i], NULL);
929+
if (has_non_ascii(buf))
930930
need_8bit_cte = 1;
931+
unuse_commit_buffer(list[i], buf);
932+
}
931933

932934
if (!branch_name)
933935
branch_name = find_branch_name(rev);
@@ -1528,8 +1530,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
15281530
reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))
15291531
die(_("Failed to create output files"));
15301532
shown = log_tree_commit(&rev, commit);
1531-
free(commit->buffer);
1532-
commit->buffer = NULL;
1533+
free_commit_buffer(commit);
15331534

15341535
/* We put one extra blank line between formatted
15351536
* patches and this flag is used by log-tree code

‎builtin/merge.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -852,8 +852,8 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
852852
parent->next->item = remoteheads->item;
853853
parent->next->next = NULL;
854854
prepare_to_commit(remoteheads);
855-
if (commit_tree(&merge_msg, result_tree, parent, result_commit, NULL,
856-
sign_commit))
855+
if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parent,
856+
result_commit, NULL, sign_commit))
857857
die(_("failed to write commit object"));
858858
finish(head, remoteheads, result_commit, "In-index merge");
859859
drop_save();
@@ -877,8 +877,8 @@ static int finish_automerge(struct commit *head,
877877
commit_list_insert(head, &parents);
878878
strbuf_addch(&merge_msg, '\n');
879879
prepare_to_commit(remoteheads);
880-
if (commit_tree(&merge_msg, result_tree, parents, result_commit,
881-
NULL, sign_commit))
880+
if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
881+
result_commit, NULL, sign_commit))
882882
die(_("failed to write commit object"));
883883
strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
884884
finish(head, remoteheads, result_commit, buf.buf);

0 commit comments

Comments
 (0)