list-objects: convert name_path to a strbuf

The "struct name_path" data is examined in only two places: we generate it in process_tree(), and we convert it to a single string in path_name(). Everyone else just passes it through to those functions. We can further note that process_tree() already keeps a single strbuf with the leading tree path, for use with tree_entry_interesting(). Instead of building a separate name_path linked list, let's just use the one we already build in "base". This reduces the amount of code (especially tricky code in path_name() which did not check for integer overflows caused by deep or large pathnames). It is also more efficient in some instances. Any time we were using tree_entry_interesting, we were building up the strbuf anyway, so this is an immediate and obvious win there. In cases where we were not, we trade off storing "pathname/" in a strbuf on the heap for each level of the path, instead of two pointers and an int on the stack (with one pointer into the tree object). On a 64-bit system, the latter is 20 bytes; so if path components are less than that on average, this has lower peak memory usage. In practice it probably doesn't matter either way; we are already holding in memory all of the tree objects leading up to each pathname, and for normal-depth pathnames, we are only talking about hundreds of bytes. This patch leaves "struct name_path" as a thin wrapper around the strbuf, to avoid disrupting callbacks. We should fix them, but leaving it out makes this diff easier to view. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 11, 2016 at 17:26 UTC 13528ab37cadb4d4f7384d0449489760912904b8
3 files changed +15 -36
list-objects.c
+9 -13
@@ -62,7 +62,6 @@ static void process_gitlink(struct rev_info *revs,
62 static void process_tree(struct rev_info *revs,
63 struct tree *tree,
64 show_object_fn show,
65 - struct name_path *path,
65 struct strbuf *base,
66 const char *name,
67 void *cb_data)
@@ -86,17 +85,14 @@ static void process_tree(struct rev_info *revs,
85 return;
86 die("bad tree object %s", oid_to_hex(&obj->oid));
87 }
88 +
89 obj->flags |= SEEN;
90 - show(obj, path, name, cb_data);
91 - me.up = path;
92 - me.elem = name;
93 - me.elem_len = strlen(name);
94 -
95 - if (!match) {
96 - strbuf_addstr(base, name);
97 - if (base->len)
98 - strbuf_addch(base, '/');
99 - }
90 + me.base = base;
91 + show(obj, &me, name, cb_data);
92 +
93 + strbuf_addstr(base, name);
94 + if (base->len)
95 + strbuf_addch(base, '/');
96
97 init_tree_desc(&desc, tree->buffer, tree->size);
98
@@ -113,7 +109,7 @@ static void process_tree(struct rev_info *revs,
109 if (S_ISDIR(entry.mode))
110 process_tree(revs,
111 lookup_tree(entry.sha1),
116 - show, &me, base, entry.path,
112 + show, base, entry.path,
113 cb_data);
114 else if (S_ISGITLINK(entry.mode))
115 process_gitlink(revs, entry.sha1,
@@ -220,7 +216,7 @@ void traverse_commit_list(struct rev_info *revs,
216 path = "";
217 if (obj->type == OBJ_TREE) {
218 process_tree(revs, (struct tree *)obj, show_object,
223 - NULL, &base, path, data);
219 + &base, path, data);
220 continue;
221 }
222 if (obj->type == OBJ_BLOB) {
revision.c
+5 -20
@@ -27,26 +27,11 @@ static const char *term_good;
27
28 char *path_name(const struct name_path *path, const char *name)
29 {
30 - const struct name_path *p;
31 - char *n, *m;
32 - int nlen = strlen(name);
33 - int len = nlen + 1;
34 -
35 - for (p = path; p; p = p->up) {
36 - if (p->elem_len)
37 - len += p->elem_len + 1;
38 - }
39 - n = xmalloc(len);
40 - m = n + len - (nlen + 1);
41 - memcpy(m, name, nlen + 1);
42 - for (p = path; p; p = p->up) {
43 - if (p->elem_len) {
44 - m -= p->elem_len + 1;
45 - memcpy(m, p->elem, p->elem_len);
46 - m[p->elem_len] = '/';
47 - }
48 - }
49 - return n;
30 + struct strbuf ret = STRBUF_INIT;
31 + if (path)
32 + strbuf_addbuf(&ret, path->base);
33 + strbuf_addstr(&ret, name);
34 + return strbuf_detach(&ret, NULL);
35 }
36
37 void show_object_with_name(FILE *out, struct object *obj,
revision.h
+1 -3
@@ -258,9 +258,7 @@ extern void mark_parents_uninteresting(struct commit *commit);
258 extern void mark_tree_uninteresting(struct tree *tree);
259
260 struct name_path {
261 - struct name_path *up;
262 - int elem_len;
263 - const char *elem;
261 + struct strbuf *base;
262 };
263
264 char *path_name(const struct name_path *path, const char *name);