Re: [PATCH 2/3] fast-import: tree allocation cleanups

["Shawn O. Pearce" <spearce@spearce.org>]

A nicely done series.  Thanks for fixing this performance bug.

Jeff King <peff@peff.net> wrote:
> @@ -1059,7 +1068,6 @@ static void load_tree(struct tree_entry *root)
>  
>  		if (t->entry_count == t->entry_capacity)
>  			root->tree = t = grow_tree_content(t, t->entry_count);
> -		t->entries[t->entry_count++] = e;
>  
>  		e->tree = NULL;
>  		c = get_mode(c, &e->versions[1].mode);
> @@ -1071,6 +1079,8 @@ static void load_tree(struct tree_entry *root)
>  		hashcpy(e->versions[0].sha1, (unsigned char*)c);
>  		hashcpy(e->versions[1].sha1, (unsigned char*)c);
>  		c += 20;
> +
> +		t->entries[t->entry_count++] = e;
>  	}
>  	free(buf);
>  }

This I wouldn't have bothered to do in this patch.  It is just
unecessary code churning, as you turn around and change this again
in the next patch.  I actually dropped these two hunks from this
patch (but left the dang commit message comment in, whoops) and
moved the first hunk to the next patch.

> @@ -1194,40 +1205,40 @@ static int tree_content_set(
>  	else
>  		n = strlen(p);
>  
> -	for (i = 0; i < t->entry_count; i++) {
> +	name = to_atom(p, n);

You missed an unsigned short cast here.

> -	e->name = to_atom(p, (unsigned short)n);
> +	e->name = name;

See?  ;-) I put the correct cast into the patch when I applied it.


Your series is applied on top of master and is now in my
fast-import.git fork on repo.or.cz.

-- 
Shawn.