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

From: Shawn O. Pearce <spearce@spearce.org>
Date: Mar 11, 2007, 03:21 UTC
Message-ID: <20070311032147.GA10781@spearce.org>
In-Reply-To: <20070310192114.GA3875@coredump.intra.peff.net>

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

Jeff King <peff@peff.net> wrote:

Show 17 quoted lines

> @@ -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.

Show 6 quoted lines

> @@ -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.