From 2d9c58c69d1bab601e67b036d0546e85abcee7eb Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 29 May 2006 12:18:33 -0700 Subject: Remove "tree->entries" tree-entry list from tree parser Instead, just use the tree buffer directly, and use the tree-walk infrastructure to walk the buffers instead of the tree-entry list. The tree-entry list is inefficient, and generates tons of small allocations for no good reason. The tree-walk infrastructure is generally no harder to use than following a linked list, and allows us to do most tree parsing in-place. Some programs still use the old tree-entry lists, and are a bit painful to convert without major surgery. For them we have a helper function that creates a temporary tree-entry list on demand. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- fetch.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'fetch.c') diff --git a/fetch.c b/fetch.c index f7f8902580..d9fe41f34f 100644 --- a/fetch.c +++ b/fetch.c @@ -41,16 +41,22 @@ static int process_tree(struct tree *tree) if (parse_tree(tree)) return -1; - entry = tree->entries; - tree->entries = NULL; + entry = create_tree_entry_list(tree); while (entry) { struct tree_entry_list *next = entry->next; - if (process(entry->item.any)) - return -1; - free(entry->name); + + if (entry->directory) { + struct tree *tree = lookup_tree(entry->sha1); + process_tree(tree); + } else { + struct blob *blob = lookup_blob(entry->sha1); + process(&blob->object); + } free(entry); entry = next; } + free(tree->buffer); + tree->buffer = NULL; return 0; } -- cgit v1.3 From 1bc995a3920da4e9143ccf9d34bdabf70ab8a211 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 29 May 2006 12:20:48 -0700 Subject: Convert fetch.c: process_tree() to raw tree walker This leaves only the horrid code in builtin-read-tree.c using the old interface. Some day I will gather the strength to tackle that one too. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- fetch.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) (limited to 'fetch.c') diff --git a/fetch.c b/fetch.c index d9fe41f34f..976a5a459c 100644 --- a/fetch.c +++ b/fetch.c @@ -3,6 +3,7 @@ #include "cache.h" #include "commit.h" #include "tree.h" +#include "tree-walk.h" #include "tag.h" #include "blob.h" #include "refs.h" @@ -36,27 +37,32 @@ static int process(struct object *obj); static int process_tree(struct tree *tree) { - struct tree_entry_list *entry; + struct tree_desc desc; if (parse_tree(tree)) return -1; - entry = create_tree_entry_list(tree); - while (entry) { - struct tree_entry_list *next = entry->next; + desc.buf = tree->buffer; + desc.size = tree->size; + while (desc.size) { + unsigned mode; + const char *name; + const unsigned char *sha1; - if (entry->directory) { - struct tree *tree = lookup_tree(entry->sha1); + sha1 = tree_entry_extract(&desc, &name, &mode); + update_tree_entry(&desc); + + if (S_ISDIR(mode)) { + struct tree *tree = lookup_tree(sha1); process_tree(tree); } else { - struct blob *blob = lookup_blob(entry->sha1); + struct blob *blob = lookup_blob(sha1); process(&blob->object); } - free(entry); - entry = next; } free(tree->buffer); tree->buffer = NULL; + tree->size = 0; return 0; } -- cgit v1.3 From 4c068a983150b740c3fcf6a33f342ac923abd3f4 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Tue, 30 May 2006 09:45:45 -0700 Subject: tree_entry(): new tree-walking helper function This adds a "tree_entry()" function that combines the common operation of doing a "tree_entry_extract()" + "update_tree_entry()". It also has a simplified calling convention, designed for simple loops that traverse over a whole tree: the arguments are pointers to the tree descriptor and a name_entry structure to fill in, and it returns a boolean "true" if there was an entry left to be gotten in the tree. This allows tree traversal with struct tree_desc desc; struct name_entry entry; desc.buf = tree->buffer; desc.size = tree->size; while (tree_entry(&desc, &entry) { ... use "entry.{path, sha1, mode, pathlen}" ... } which is not only shorter than writing it out in full, it's hopefully less error prone too. [ It's actually a tad faster too - we don't need to recalculate the entry pathlength in both extract and update, but need to do it only once. Also, some callers can avoid doing a "strlen()" on the result, since it's returned as part of the name_entry structure. However, by now we're talking just 1% speedup on "git-rev-list --objects --all", and we're definitely at the point where tree walking is no longer the issue any more. ] NOTE! Not everybody wants to use this new helper function, since some of the tree walkers very much on purpose do the descriptor update separately from the entry extraction. So the "extract + update" sequence still remains as the core sequence, this is just a simplified interface. We should probably add a silly two-line inline helper function for initializing the descriptor from the "struct tree" too, just to cut down on the noise from that common "desc" initializer. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- builtin-grep.c | 26 ++++++++++---------------- builtin-read-tree.c | 36 +++++++++++++----------------------- builtin-rev-list.c | 16 +++++----------- builtin-tar-tree.c | 21 ++++++++------------- fetch.c | 16 +++++----------- http-push.c | 16 +++++----------- pack-objects.c | 27 +++++++++++---------------- revision.c | 16 +++++----------- tree-walk.c | 33 +++++++++++++++++++++++++++++++-- tree-walk.h | 5 ++++- tree.c | 41 +++++++++++++++-------------------------- 11 files changed, 112 insertions(+), 141 deletions(-) (limited to 'fetch.c') diff --git a/builtin-grep.c b/builtin-grep.c index 53de8a8836..acc4eea363 100644 --- a/builtin-grep.c +++ b/builtin-grep.c @@ -578,11 +578,9 @@ static int grep_tree(struct grep_opt *opt, const char **paths, struct tree_desc *tree, const char *tree_name, const char *base) { - unsigned mode; int len; int hit = 0; - const char *path; - const unsigned char *sha1; + struct name_entry entry; char *down; char *path_buf = xmalloc(PATH_MAX + strlen(tree_name) + 100); @@ -597,36 +595,32 @@ static int grep_tree(struct grep_opt *opt, const char **paths, } len = strlen(path_buf); - while (tree->size) { - int pathlen; - sha1 = tree_entry_extract(tree, &path, &mode); - pathlen = strlen(path); - strcpy(path_buf + len, path); + while (tree_entry(tree, &entry)) { + strcpy(path_buf + len, entry.path); - if (S_ISDIR(mode)) + if (S_ISDIR(entry.mode)) /* Match "abc/" against pathspec to * decide if we want to descend into "abc" * directory. */ - strcpy(path_buf + len + pathlen, "/"); + strcpy(path_buf + len + entry.pathlen, "/"); if (!pathspec_matches(paths, down)) ; - else if (S_ISREG(mode)) - hit |= grep_sha1(opt, sha1, path_buf); - else if (S_ISDIR(mode)) { + else if (S_ISREG(entry.mode)) + hit |= grep_sha1(opt, entry.sha1, path_buf); + else if (S_ISDIR(entry.mode)) { char type[20]; struct tree_desc sub; void *data; - data = read_sha1_file(sha1, type, &sub.size); + data = read_sha1_file(entry.sha1, type, &sub.size); if (!data) die("unable to read tree (%s)", - sha1_to_hex(sha1)); + sha1_to_hex(entry.sha1)); sub.buf = data; hit |= grep_tree(opt, paths, &sub, tree_name, down); free(data); } - update_tree_entry(tree); } return hit; } diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 00cdb5a6d9..10afd46968 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -53,28 +53,23 @@ typedef int (*merge_fn_t)(struct cache_entry **src); static struct tree_entry_list *create_tree_entry_list(struct tree *tree) { struct tree_desc desc; + struct name_entry one; struct tree_entry_list *ret = NULL; struct tree_entry_list **list_p = &ret; desc.buf = tree->buffer; desc.size = tree->size; - while (desc.size) { - unsigned mode; - const char *path; - const unsigned char *sha1; + while (tree_entry(&desc, &one)) { struct tree_entry_list *entry; - sha1 = tree_entry_extract(&desc, &path, &mode); - update_tree_entry(&desc); - entry = xmalloc(sizeof(struct tree_entry_list)); - entry->name = path; - entry->sha1 = sha1; - entry->mode = mode; - entry->directory = S_ISDIR(mode) != 0; - entry->executable = (mode & S_IXUSR) != 0; - entry->symlink = S_ISLNK(mode) != 0; + entry->name = one.path; + entry->sha1 = one.sha1; + entry->mode = one.mode; + entry->directory = S_ISDIR(one.mode) != 0; + entry->executable = (one.mode & S_IXUSR) != 0; + entry->symlink = S_ISLNK(one.mode) != 0; entry->next = NULL; *list_p = entry; @@ -820,27 +815,22 @@ static int read_cache_unmerged(void) static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree) { struct tree_desc desc; + struct name_entry entry; int cnt; memcpy(it->sha1, tree->object.sha1, 20); desc.buf = tree->buffer; desc.size = tree->size; cnt = 0; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; - - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - if (!S_ISDIR(mode)) + while (tree_entry(&desc, &entry)) { + if (!S_ISDIR(entry.mode)) cnt++; else { struct cache_tree_sub *sub; - struct tree *subtree = lookup_tree(sha1); + struct tree *subtree = lookup_tree(entry.sha1); if (!subtree->object.parsed) parse_tree(subtree); - sub = cache_tree_sub(it, name); + sub = cache_tree_sub(it, entry.path); sub->cache_tree = cache_tree(); prime_cache_tree_rec(sub->cache_tree, subtree); cnt += sub->cache_tree->entry_count; diff --git a/builtin-rev-list.c b/builtin-rev-list.c index 6e2b898cca..17c04b962d 100644 --- a/builtin-rev-list.c +++ b/builtin-rev-list.c @@ -114,6 +114,7 @@ static struct object_list **process_tree(struct tree *tree, { struct object *obj = &tree->object; struct tree_desc desc; + struct name_entry entry; struct name_path me; if (!revs.tree_objects) @@ -132,18 +133,11 @@ static struct object_list **process_tree(struct tree *tree, desc.buf = tree->buffer; desc.size = tree->size; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; - - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - - if (S_ISDIR(mode)) - p = process_tree(lookup_tree(sha1), p, &me, name); + while (tree_entry(&desc, &entry)) { + if (S_ISDIR(entry.mode)) + p = process_tree(lookup_tree(entry.sha1), p, &me, name); else - p = process_blob(lookup_blob(sha1), p, &me, name); + p = process_blob(lookup_blob(entry.sha1), p, &me, name); } free(tree->buffer); tree->buffer = NULL; diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c index 2d5e06fb69..5f740cf702 100644 --- a/builtin-tar-tree.c +++ b/builtin-tar-tree.c @@ -271,30 +271,25 @@ static void write_global_extended_header(const unsigned char *sha1) static void traverse_tree(struct tree_desc *tree, struct strbuf *path) { int pathlen = path->len; + struct name_entry entry; - while (tree->size) { - const char *name; - const unsigned char *sha1; - unsigned mode; + while (tree_entry(tree, &entry)) { void *eltbuf; char elttype[20]; unsigned long eltsize; - sha1 = tree_entry_extract(tree, &name, &mode); - update_tree_entry(tree); - - eltbuf = read_sha1_file(sha1, elttype, &eltsize); + eltbuf = read_sha1_file(entry.sha1, elttype, &eltsize); if (!eltbuf) - die("cannot read %s", sha1_to_hex(sha1)); + die("cannot read %s", sha1_to_hex(entry.sha1)); path->len = pathlen; - strbuf_append_string(path, name); - if (S_ISDIR(mode)) + strbuf_append_string(path, entry.path); + if (S_ISDIR(entry.mode)) strbuf_append_string(path, "/"); - write_entry(sha1, path, mode, eltbuf, eltsize); + write_entry(entry.sha1, path, entry.mode, eltbuf, eltsize); - if (S_ISDIR(mode)) { + if (S_ISDIR(entry.mode)) { struct tree_desc subtree; subtree.buf = eltbuf; subtree.size = eltsize; diff --git a/fetch.c b/fetch.c index 976a5a459c..ec2d8c3d9b 100644 --- a/fetch.c +++ b/fetch.c @@ -38,25 +38,19 @@ static int process(struct object *obj); static int process_tree(struct tree *tree) { struct tree_desc desc; + struct name_entry entry; if (parse_tree(tree)) return -1; desc.buf = tree->buffer; desc.size = tree->size; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; - - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - - if (S_ISDIR(mode)) { - struct tree *tree = lookup_tree(sha1); + while (tree_entry(&desc, &entry)) { + if (S_ISDIR(entry.mode)) { + struct tree *tree = lookup_tree(entry.sha1); process_tree(tree); } else { - struct blob *blob = lookup_blob(sha1); + struct blob *blob = lookup_blob(entry.sha1); process(&blob->object); } } diff --git a/http-push.c b/http-push.c index 72ad89ce11..b1c018a089 100644 --- a/http-push.c +++ b/http-push.c @@ -1715,6 +1715,7 @@ static struct object_list **process_tree(struct tree *tree, { struct object *obj = &tree->object; struct tree_desc desc; + struct name_entry entry; struct name_path me; obj->flags |= LOCAL; @@ -1734,18 +1735,11 @@ static struct object_list **process_tree(struct tree *tree, desc.buf = tree->buffer; desc.size = tree->size; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; - - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - - if (S_ISDIR(mode)) - p = process_tree(lookup_tree(sha1), p, &me, name); + while (tree_entry(&desc, &entry)) { + if (S_ISDIR(entry.mode)) + p = process_tree(lookup_tree(entry.sha1), p, &me, name); else - p = process_blob(lookup_blob(sha1), p, &me, name); + p = process_blob(lookup_blob(entry.sha1), p, &me, name); } free(tree->buffer); tree->buffer = NULL; diff --git a/pack-objects.c b/pack-objects.c index 77284cfdba..3590cd5e59 100644 --- a/pack-objects.c +++ b/pack-objects.c @@ -690,25 +690,20 @@ static void add_pbase_object(struct tree_desc *tree, const char *name, int cmplen) { - while (tree->size) { - const unsigned char *sha1; - const char *entry_name; - int entry_len; - unsigned mode; + struct name_entry entry; + + while (tree_entry(tree,&entry)) { unsigned long size; char type[20]; - sha1 = tree_entry_extract(tree, &entry_name, &mode); - update_tree_entry(tree); - entry_len = strlen(entry_name); - if (entry_len != cmplen || - memcmp(entry_name, name, cmplen) || - !has_sha1_file(sha1) || - sha1_object_info(sha1, type, &size)) + if (entry.pathlen != cmplen || + memcmp(entry.path, name, cmplen) || + !has_sha1_file(entry.sha1) || + sha1_object_info(entry.sha1, type, &size)) continue; if (name[cmplen] != '/') { unsigned hash = name_hash(up, name); - add_object_entry(sha1, hash, 1); + add_object_entry(entry.sha1, hash, 1); return; } if (!strcmp(type, tree_type)) { @@ -718,15 +713,15 @@ static void add_pbase_object(struct tree_desc *tree, const char *down = name+cmplen+1; int downlen = name_cmp_len(down); - tree = pbase_tree_get(sha1); + tree = pbase_tree_get(entry.sha1); if (!tree) return; sub.buf = tree->tree_data; sub.size = tree->tree_size; me.up = up; - me.elem = entry_name; - me.len = entry_len; + me.elem = entry.path; + me.len = entry.pathlen; add_pbase_object(&sub, &me, down, downlen); pbase_tree_put(tree); } diff --git a/revision.c b/revision.c index 8e93e40bb3..6a6952cd55 100644 --- a/revision.c +++ b/revision.c @@ -54,6 +54,7 @@ static void mark_blob_uninteresting(struct blob *blob) void mark_tree_uninteresting(struct tree *tree) { struct tree_desc desc; + struct name_entry entry; struct object *obj = &tree->object; if (obj->flags & UNINTERESTING) @@ -66,18 +67,11 @@ void mark_tree_uninteresting(struct tree *tree) desc.buf = tree->buffer; desc.size = tree->size; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; - - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - - if (S_ISDIR(mode)) - mark_tree_uninteresting(lookup_tree(sha1)); + while (tree_entry(&desc, &entry)) { + if (S_ISDIR(entry.mode)) + mark_tree_uninteresting(lookup_tree(entry.sha1)); else - mark_blob_uninteresting(lookup_blob(sha1)); + mark_blob_uninteresting(lookup_blob(entry.sha1)); } /* diff --git a/tree-walk.c b/tree-walk.c index 3922058271..297c6972b9 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -37,7 +37,7 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a) void update_tree_entry(struct tree_desc *desc) { - void *buf = desc->buf; + const void *buf = desc->buf; unsigned long size = desc->size; int len = strlen(buf) + 1 + 20; @@ -63,7 +63,7 @@ static const char *get_mode(const char *str, unsigned int *modep) const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep) { - void *tree = desc->buf; + const void *tree = desc->buf; unsigned long size = desc->size; int len = strlen(tree)+1; const unsigned char *sha1 = tree + len; @@ -78,6 +78,35 @@ const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pat return sha1; } +int tree_entry(struct tree_desc *desc, struct name_entry *entry) +{ + const void *tree = desc->buf, *path; + unsigned long len, size = desc->size; + + if (!size) + return 0; + + path = get_mode(tree, &entry->mode); + if (!path) + die("corrupt tree file"); + + entry->path = path; + len = strlen(path); + entry->pathlen = len; + + path += len + 1; + entry->sha1 = path; + + path += 20; + len = path - tree; + if (len > size) + die("corrupt tree file"); + + desc->buf = path; + desc->size = size - len; + return 1; +} + void traverse_trees(int n, struct tree_desc *t, const char *base, traverse_callback_t callback) { struct name_entry *entry = xmalloc(n*sizeof(*entry)); diff --git a/tree-walk.h b/tree-walk.h index 47438fe1c0..e57befa4da 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -2,7 +2,7 @@ #define TREE_WALK_H struct tree_desc { - void *buf; + const void *buf; unsigned long size; }; @@ -16,6 +16,9 @@ struct name_entry { void update_tree_entry(struct tree_desc *); const unsigned char *tree_entry_extract(struct tree_desc *, const char **, unsigned int *); +/* Helper function that does both of the above and returns true for success */ +int tree_entry(struct tree_desc *, struct name_entry *); + void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1); typedef void (*traverse_callback_t)(int n, unsigned long mask, struct name_entry *entry, const char *base); diff --git a/tree.c b/tree.c index fb18724259..9bbe2da37b 100644 --- a/tree.c +++ b/tree.c @@ -79,6 +79,7 @@ int read_tree_recursive(struct tree *tree, read_tree_fn_t fn) { struct tree_desc desc; + struct name_entry entry; if (parse_tree(tree)) return -1; @@ -86,18 +87,11 @@ int read_tree_recursive(struct tree *tree, desc.buf = tree->buffer; desc.size = tree->size; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; - - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - - if (!match_tree_entry(base, baselen, name, mode, match)) + while (tree_entry(&desc, &entry)) { + if (!match_tree_entry(base, baselen, entry.path, entry.mode, match)) continue; - switch (fn(sha1, base, baselen, name, mode, stage)) { + switch (fn(entry.sha1, base, baselen, entry.path, entry.mode, stage)) { case 0: continue; case READ_TREE_RECURSIVE: @@ -105,18 +99,17 @@ int read_tree_recursive(struct tree *tree, default: return -1; } - if (S_ISDIR(mode)) { + if (S_ISDIR(entry.mode)) { int retval; - int pathlen = strlen(name); char *newbase; - newbase = xmalloc(baselen + 1 + pathlen); + newbase = xmalloc(baselen + 1 + entry.pathlen); memcpy(newbase, base, baselen); - memcpy(newbase + baselen, name, pathlen); - newbase[baselen + pathlen] = '/'; - retval = read_tree_recursive(lookup_tree(sha1), + memcpy(newbase + baselen, entry.path, entry.pathlen); + newbase[baselen + entry.pathlen] = '/'; + retval = read_tree_recursive(lookup_tree(entry.sha1), newbase, - baselen + pathlen + 1, + baselen + entry.pathlen + 1, stage, match, fn); free(newbase); if (retval) @@ -156,6 +149,7 @@ static int track_tree_refs(struct tree *item) int n_refs = 0, i; struct object_refs *refs; struct tree_desc desc; + struct name_entry entry; /* Count how many entries there are.. */ desc.buf = item->buffer; @@ -170,18 +164,13 @@ static int track_tree_refs(struct tree *item) refs = alloc_object_refs(n_refs); desc.buf = item->buffer; desc.size = item->size; - while (desc.size) { - unsigned mode; - const char *name; - const unsigned char *sha1; + while (tree_entry(&desc, &entry)) { struct object *obj; - sha1 = tree_entry_extract(&desc, &name, &mode); - update_tree_entry(&desc); - if (S_ISDIR(mode)) - obj = &lookup_tree(sha1)->object; + if (S_ISDIR(entry.mode)) + obj = &lookup_tree(entry.sha1)->object; else - obj = &lookup_blob(sha1)->object; + obj = &lookup_blob(entry.sha1)->object; refs->ref[i++] = obj; } set_object_refs(&item->object, refs); -- cgit v1.3 From 6f9012b62517ca490e4131f24e03ff842527f1b9 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 2 Jun 2006 15:23:47 -0700 Subject: fetch.c: do not call process_tree() from process_tree(). This function reads a freshly fetched tree object, and schedules the objects pointed by it for further fetching, so doing lookup_tree() and process_tree() recursively from there does not make much sense. We need to use process() on it to make sure we fetch it first, and leave the recursive processing to later stages. Signed-off-by: Junio C Hamano --- fetch.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'fetch.c') diff --git a/fetch.c b/fetch.c index ec2d8c3d9b..107504b72b 100644 --- a/fetch.c +++ b/fetch.c @@ -46,13 +46,20 @@ static int process_tree(struct tree *tree) desc.buf = tree->buffer; desc.size = tree->size; while (tree_entry(&desc, &entry)) { + struct object *obj = NULL; + if (S_ISDIR(entry.mode)) { struct tree *tree = lookup_tree(entry.sha1); - process_tree(tree); - } else { + if (tree) + obj = &tree->object; + } + else { struct blob *blob = lookup_blob(entry.sha1); - process(&blob->object); + if (blob) + obj = &blob->object; } + if (!obj || process(obj)) + return -1; } free(tree->buffer); tree->buffer = NULL; -- cgit v1.3