From: Ben Peart Date: Mon, 30 Jul 2018 20:58:28 GMT Subject: Re: [PATCH v2 3/4] unpack-trees: reduce malloc in cache-tree walk Message-ID: <0cbba28a-1820-63bf-b31c-cb3774d7d5bd@gmail.com> In-Reply-To: <20180729103306.16403-4-pclouds@gmail.com> On 7/29/2018 6:33 AM, Nguyễn Thái Ngọc Duy wrote: > This is a micro optimization that probably only shines on repos with > deep directory structure. Instead of allocating and freeing a new > cache_entry in every iteration, we reuse the last one and only update > the parts that are new each iteration. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > unpack-trees.c | 29 ++++++++++++++++++++--------- > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/unpack-trees.c b/unpack-trees.c > index 39566b28fb..c33ebaf001 100644 > --- a/unpack-trees.c > +++ b/unpack-trees.c > @@ -694,6 +694,8 @@ static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names, > { > struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, }; > struct unpack_trees_options *o = info->data; > + struct cache_entry *tree_ce = NULL; > + int ce_len = 0; > int i, d; > > if (!o->merge) > @@ -708,30 +710,39 @@ static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names, > * in the first place. > */ > for (i = 0; i < nr_entries; i++) { > - struct cache_entry *tree_ce; > - int len, rc; > + int new_ce_len, len, rc; > > src[0] = o->src_index->cache[pos + i]; > > len = ce_namelen(src[0]); > - tree_ce = xcalloc(1, cache_entry_size(len)); > + new_ce_len = cache_entry_size(len); > + > + if (new_ce_len > ce_len) { > + new_ce_len <<= 1; > + tree_ce = xrealloc(tree_ce, new_ce_len); > + memset(tree_ce, 0, new_ce_len); > + ce_len = new_ce_len; > + > + tree_ce->ce_flags = create_ce_flags(0); > + > + for (d = 1; d <= nr_names; d++) > + src[d] = tree_ce; > + } Nice optimization - especially when there are a lot of cache entries and large trees. > > tree_ce->ce_mode = src[0]->ce_mode; > - tree_ce->ce_flags = create_ce_flags(0); > tree_ce->ce_namelen = len; > oidcpy(&tree_ce->oid, &src[0]->oid); > memcpy(tree_ce->name, src[0]->name, len + 1); > > - for (d = 1; d <= nr_names; d++) > - src[d] = tree_ce; > - > rc = call_unpack_fn((const struct cache_entry * const *)src, o); > - free(tree_ce); > - if (rc < 0) > + if (rc < 0) { > + free(tree_ce); > return rc; > + } > > mark_ce_used(src[0], o); > } > + free(tree_ce); > if (o->debug_unpack) > printf("Unpacked %d entries from %s to %s using cache-tree\n", > nr_entries, >