Typically every commit has at least one struct commit_list allocated for its parent. It is also not uncommon for there to be more than one allocated per commit, especially if merges are common in this repository. Running `git rev-list --all` doesn't usually wind up requiring many commit_list items (about 1024 for git.git right now) as the commits are output as soon as they are first seen and then the parent list is freed as it is no longer necessary for traversal. However when we add --topo-order or --date-order and request sorting the lists are not deallocated until after sorting is complete. Any memory they consume during that time just increases our footprint. In git.git for 15865 commits we need 50354 commit_lists to complete a --topo-order sort. That's an average of 3 commit_list items per commit (I blame my local reflogs of Junio's pu branch for the high ratio of commits to their parents). With that kind of allocation individually mallocing them is wasting a lot of memory for these very small and otherwise lightwight objects. Aside from reducing our memory usage during the --topo-order sorting case a major benefit from allocating these by blocks is we can release them enmass when we release the other objects in the object pool. This reduces the cost of the scrub_commit() function as it no longer needs to free each parent entry individually. Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx> --- alloc.c | 5 ++++- builtin-diff-tree.c | 8 ++------ builtin-rev-parse.c | 2 +- builtin-send-pack.c | 2 +- builtin-show-branch.c | 2 +- cache.h | 2 ++ commit.c | 13 ++++++------- http-push.c | 2 +- reflog-walk.c | 2 +- revision.c | 6 +++--- upload-pack.c | 2 +- 11 files changed, 23 insertions(+), 23 deletions(-) diff --git a/alloc.c b/alloc.c index 5737a73..b69f613 100644 --- a/alloc.c +++ b/alloc.c @@ -51,7 +51,7 @@ static void report(const char* name, struct node_pool *p, size_t sz) count -= p->nr; sz = (count * sz) >> 10; - fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)", name, count, sz); + fprintf(stderr, "%15s: %8u (" SZ_FMT " kB)", name, count, sz); if (free) fprintf(stderr, ", %8u on free list", free); fputc('\n', stderr); @@ -144,6 +144,7 @@ static void scrub_any_object(union any_object *o) } } +DEFINE_ALLOCATOR(commit_list, struct commit_list) DEFINE_ALLOCATOR(blob, struct blob) DEFINE_SCRUBBING_ALLOCATOR(tree, struct tree, scrub_tree) DEFINE_SCRUBBING_ALLOCATOR(commit, struct commit, scrub_commit) @@ -155,6 +156,7 @@ void alloc_report(void) report_blob(); report_tree(); report_commit(); + report_commit_list(); report_tag(); report_object(); } @@ -164,6 +166,7 @@ void alloc_free_everything(void) free_all_blob(); free_all_tree(); free_all_commit(); + free_all_commit_list(); free_all_tag(); free_all_object(); } diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c index 0b591c8..eb340d2 100644 --- a/builtin-diff-tree.c +++ b/builtin-diff-tree.c @@ -31,14 +31,10 @@ static int diff_tree_stdin(char *line) if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) { /* Graft the fake parents locally to the commit */ int pos = 41; - struct commit_list **pptr, *parents; + struct commit_list **pptr; /* Free the real parent list */ - for (parents = commit->parents; parents; ) { - struct commit_list *tmp = parents->next; - free(parents); - parents = tmp; - } + free_commit_list(commit->parents); commit->parents = NULL; pptr = &(commit->parents); while (line[pos] && !get_sha1_hex(line + pos, sha1)) { diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c index d1038a0..7500814 100644 --- a/builtin-rev-parse.c +++ b/builtin-rev-parse.c @@ -200,7 +200,7 @@ static int try_difference(const char *arg) struct commit_list *n = exclude->next; show_rev(REVERSED, exclude->item->object.sha1,NULL); - free(exclude); + free_commit_list_node(exclude); exclude = n; } } diff --git a/builtin-send-pack.c b/builtin-send-pack.c index 5a0f5c6..818519e 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -82,7 +82,7 @@ static void unmark_and_free(struct commit_list *list, unsigned int mark) struct commit_list *temp = list; temp->item->object.flags &= ~mark; list = temp->next; - free(temp); + free_commit_list_node(temp); } } diff --git a/builtin-show-branch.c b/builtin-show-branch.c index 6dc835d..f10377c 100644 --- a/builtin-show-branch.c +++ b/builtin-show-branch.c @@ -38,7 +38,7 @@ static struct commit *pop_one_commit(struct commit_list **list_p) list = *list_p; commit = list->item; *list_p = list->next; - free(list); + free_commit_list_node(list); return commit; } diff --git a/cache.h b/cache.h index 8289de1..a33c8ee 100644 --- a/cache.h +++ b/cache.h @@ -586,6 +586,8 @@ extern void *alloc_tree_node(void); extern void *alloc_commit_node(void); extern void *alloc_tag_node(void); extern void *alloc_object_node(void); +extern void *alloc_commit_list_node(void); +extern void free_commit_list_node(void *); extern void alloc_report(void); extern void alloc_free_everything(void); diff --git a/commit.c b/commit.c index 05f3cd6..796fc59 100644 --- a/commit.c +++ b/commit.c @@ -320,13 +320,12 @@ int parse_commit(struct commit *item) void scrub_commit(struct commit *item) { - free_commit_list(item->parents); free(item->buffer); } struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p) { - struct commit_list *new_list = xmalloc(sizeof(struct commit_list)); + struct commit_list *new_list = alloc_commit_list_node(); new_list->item = item; new_list->next = *list_p; *list_p = new_list; @@ -338,7 +337,7 @@ void free_commit_list(struct commit_list *list) while (list) { struct commit_list *temp = list; list = temp->next; - free(temp); + free_commit_list_node(temp); } } @@ -374,7 +373,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list, struct commit_list *old = *list; *list = (*list)->next; - free(old); + free_commit_list_node(old); while (parents) { struct commit *commit = parents->item; @@ -416,7 +415,7 @@ struct commit *pop_commit(struct commit_list **stack) if (top) { *stack = top->next; - free(top); + free_commit_list_node(top); } return item; } @@ -561,7 +560,7 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two) commit = list->item; n = list->next; - free(list); + free_commit_list_node(list); list = n; flags = commit->object.flags & (PARENT1 | PARENT2 | STALE); @@ -592,7 +591,7 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two) struct commit_list *n = list->next; if (!(list->item->object.flags & STALE)) insert_by_date(list->item, &result); - free(list); + free_commit_list_node(list); list = n; } return result; diff --git a/http-push.c b/http-push.c index 99328f5..75f1664 100644 --- a/http-push.c +++ b/http-push.c @@ -1931,7 +1931,7 @@ static void unmark_and_free(struct commit_list *list, unsigned int mark) struct commit_list *temp = list; temp->item->object.flags &= ~mark; list = temp->next; - free(temp); + free_commit_list_node(temp); } } diff --git a/reflog-walk.c b/reflog-walk.c index ee1456b..a6cc6ca 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -236,7 +236,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit) return; } - commit->parents = xcalloc(sizeof(struct commit_list), 1); + commit->parents = alloc_commit_list_node(); commit->parents->item = commit_info->commit; commit->object.flags &= ~(ADDED | SEEN | SHOWN); } diff --git a/revision.c b/revision.c index 931f978..d9c3998 100644 --- a/revision.c +++ b/revision.c @@ -554,7 +554,7 @@ static int limit_list(struct rev_info *revs) show_early_output_fn_t show; list = list->next; - free(entry); + free_commit_list_node(entry); if (revs->max_age != -1 && (commit->date < revs->max_age)) obj->flags |= UNINTERESTING; @@ -735,7 +735,7 @@ static void prepare_show_merge(struct rev_info *revs) while (bases) { struct commit *it = bases->item; struct commit_list *n = bases->next; - free(bases); + free_commit_list_node(bases); bases = n; it->object.flags |= UNINTERESTING; add_pending_object(revs, &it->object, "(merge-base)"); @@ -1451,7 +1451,7 @@ static struct commit *get_revision_1(struct rev_info *revs) struct commit *commit = entry->item; revs->commits = entry->next; - free(entry); + free_commit_list_node(entry); if (revs->reflog_info) fake_reflog_parent(revs->reflog_info, commit); diff --git a/upload-pack.c b/upload-pack.c index 7e04311..51796dc 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -331,7 +331,7 @@ static int reachable(struct commit *want) while (work) { struct commit_list *list = work->next; struct commit *commit = work->item; - free(work); + free_commit_list_node(work); work = list; if (commit->object.flags & THEY_HAVE) { -- 1.5.3.5.1622.g41d10 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html