tree: store the entries in a growable arraycmn/tree-reuse

Take advantage of the constant size of tree-owned arrays and store them
in an array instead of a pool. This still lets us free them all at once
but lets the system allocator do the work of fitting them in.

2 files changed, 38 insertions, 60 deletions

diff --git a/src/tree.c b/src/tree.c
index a6bd7d4fb..1ba484918 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -85,30 +85,26 @@ int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2)
 }
 
 /**
- * Allocate either from the pool or from the system allocator
+ * Allocate a new self-contained entry, with enough space after it to
+ * store the filename and the id.
  */
-static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, size_t filename_len, const git_oid *id)
+static git_tree_entry *alloc_entry(const char *filename, size_t filename_len, const git_oid *id)
 {
 	git_tree_entry *entry = NULL;
 	size_t tree_len;
 
 	TREE_ENTRY_CHECK_NAMELEN(filename_len);
-	tree_len = sizeof(git_tree_entry);
 
-	if (!pool && (GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, filename_len) ||
-		      GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1) ||
-		      GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, GIT_OID_RAWSZ)))
+	if (GIT_ADD_SIZET_OVERFLOW(&tree_len, sizeof(git_tree_entry), filename_len) ||
+	    GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1) ||
+	    GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, GIT_OID_RAWSZ))
 		return NULL;
 
-	entry = pool ? git_pool_mallocz(pool, tree_len) :
-		       git__calloc(1, tree_len);
+	entry = git__calloc(1, tree_len);
 	if (!entry)
 		return NULL;
 
-	if (pool) {
-		entry->filename = filename;
-		entry->oid = (git_oid *) id;
-	} else {
+	{
 		char *filename_ptr;
 		void *id_ptr;
 
@@ -126,29 +122,6 @@ static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, si
 	return entry;
 }
 
-/**
- * Allocate a tree entry, using the poolin the tree which owns
- * it. This is useful when reading trees, so we don't allocate a ton
- * of small strings but can use the pool.
- */
-static git_tree_entry *alloc_entry_pooled(git_pool *pool, const char *filename, size_t filename_len, const git_oid *id)
-{
-	git_tree_entry *entry = NULL;
-
-	if (!(entry = alloc_entry_base(pool, filename, filename_len, id)))
-		return NULL;
-
-	entry->pooled = true;
-
-	return entry;
-}
-
-static git_tree_entry *alloc_entry(const char *filename)
-{
-	git_oid dummy_id = { 0 };
-	return alloc_entry_base(NULL, filename, strlen(filename), &dummy_id);
-}
-
 struct tree_key_search {
 	const char *filename;
 	uint16_t filename_len;
@@ -250,7 +223,7 @@ static int tree_key_search(
 
 void git_tree_entry_free(git_tree_entry *entry)
 {
-	if (entry == NULL || entry->pooled)
+	if (entry == NULL)
 		return;
 
 	git__free(entry);
@@ -258,27 +231,27 @@ void git_tree_entry_free(git_tree_entry *entry)
 
 int git_tree_entry_dup(git_tree_entry **dest, const git_tree_entry *source)
 {
+	git_tree_entry *cpy;
+
 	assert(source);
 
-	*dest = alloc_entry_base(NULL, source->filename, source->filename_len, source->oid);
-	if (*dest == NULL)
+	cpy = alloc_entry(source->filename, source->filename_len, source->oid);
+	if (cpy == NULL)
 		return -1;
 
+	cpy->attr = source->attr;
+
+	*dest = cpy;
 	return 0;
 }
 
 void git_tree__free(void *_tree)
 {
 	git_tree *tree = _tree;
-	size_t i;
-	git_tree_entry *e;
-
-	git_vector_foreach(&tree->entries, i, e)
-		git_tree_entry_free(e);
 
 	git_odb_object_free(tree->odb_obj);
 	git_vector_free(&tree->entries);
-	git_pool_clear(&tree->pool);
+	git_array_clear(tree->entries_arr);
 	git__free(tree);
 }
 
@@ -450,6 +423,7 @@ static int parse_mode(unsigned int *modep, const char *buffer, const char **buff
 
 int git_tree__parse(void *_tree, git_odb_object *odb_obj)
 {
+	size_t i;
 	git_tree *tree = _tree;
 	const char *buffer;
 	const char *buffer_end;
@@ -460,9 +434,8 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj)
 	buffer = git_odb_object_data(tree->odb_obj);
 	buffer_end = buffer + git_odb_object_size(tree->odb_obj);
 
-	git_pool_init(&tree->pool, 1);
-	if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0)
-		return -1;
+	git_array_init_to_size(tree->entries_arr, DEFAULT_TREE_SIZE);
+	GITERR_CHECK_ARRAY(tree->entries_arr);
 
 	while (buffer < buffer_end) {
 		git_tree_entry *entry;
@@ -479,21 +452,28 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj)
 		filename_len = nul - buffer;
 		/** Allocate the entry and store it in the entries vector */
 		{
-			/* Jump to the ID just after the path */
-			const void *oid_ptr = buffer + filename_len + 1;
-			entry = alloc_entry_pooled(&tree->pool, buffer, filename_len, oid_ptr);
+			entry = git_array_alloc(tree->entries_arr);
 			GITERR_CHECK_ALLOC(entry);
 
-			if (git_vector_insert(&tree->entries, entry) < 0)
-				return -1;
-
 			entry->attr = attr;
+			entry->filename_len = filename_len;
+			entry->filename = buffer;
+			entry->oid = (git_oid *) ((char *) buffer + filename_len + 1);
 		}
 
 		buffer += filename_len + 1;
 		buffer += GIT_OID_RAWSZ;
 	}
 
+	/* Add the entries to the vector here, as we may reallocate during the loop */
+	if (git_vector_init(&tree->entries, tree->entries_arr.size, entry_sort_cmp) < 0)
+		return -1;
+
+	for (i = 0; i < tree->entries_arr.size; i++) {
+		if (git_vector_insert(&tree->entries, git_array_get(tree->entries_arr, i)) < 0)
+			return -1;
+	}
+
 	/* The tree is sorted by definition. Bad inputs give bad outputs */
 	tree->entries.flags |= GIT_VECTOR_SORTED;
 
@@ -529,10 +509,9 @@ static int append_entry(
 	if (!valid_entry_name(bld->repo, filename))
 		return tree_error("Failed to insert entry. Invalid name for a tree entry", filename);
 
-	entry = alloc_entry(filename);
+	entry = alloc_entry(filename, strlen(filename), id);
 	GITERR_CHECK_ALLOC(entry);
 
-	git_oid_cpy(entry->oid, id);
 	entry->attr = (uint16_t)filemode;
 
 	git_strmap_insert(bld->map, entry->filename, entry, error);
@@ -776,8 +755,9 @@ int git_treebuilder_insert(
 	pos = git_strmap_lookup_index(bld->map, filename);
 	if (git_strmap_valid_index(bld->map, pos)) {
 		entry = git_strmap_value_at(bld->map, pos);
+		git_oid_cpy((git_oid *) entry->oid, id);
 	} else {
-		entry = alloc_entry(filename);
+		entry = alloc_entry(filename, strlen(filename), id);
 		GITERR_CHECK_ALLOC(entry);
 
 		git_strmap_insert(bld->map, entry->filename, entry, error);
@@ -789,7 +769,6 @@ int git_treebuilder_insert(
 		}
 	}
 
-	git_oid_cpy(entry->oid, id);
 	entry->attr = filemode;
 
 	if (entry_out)
diff --git a/src/tree.h b/src/tree.h
index 00b27ae41..a108d964c 100644
--- a/src/tree.h
+++ b/src/tree.h
@@ -17,16 +17,15 @@
 struct git_tree_entry {
 	uint16_t attr;
 	uint16_t filename_len;
-	git_oid *oid;
-	bool pooled;
+	const git_oid *oid;
 	const char *filename;
 };
 
 struct git_tree {
 	git_object object;
 	git_odb_object *odb_obj;
+	git_array_t(git_tree_entry) entries_arr;
 	git_vector entries;
-	git_pool pool;
 };
 
 struct git_treebuilder {