Re: [PATCH 1/3] use struct sha1_array in diff_tree_combined()

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 17.12.2011 11:53, schrieb Jeff King:
>> -	parent = xmalloc(ents * sizeof(*parent));
>> -	for (i = 0; i<  ents; i++)
>> -		hashcpy((unsigned char *)(parent + i), ent[i].item->sha1);
>> -	diff_tree_combined(parent[0], parent + 1, ents - 1,
>> +	for (i = 1; i<  ents; i++)
>> +		sha1_array_append(&parents, ent[i].item->sha1);
>> +	diff_tree_combined(ent[0].item->sha1,&parents,
>>   			   revs->dense_combined_merges, revs);
>> -	free((void *)parent);
>> +	sha1_array_clear(&parents);
>
> The original code is slightly more efficient, as it is able to use a
> single malloc (because it knows the number of entries ahead of time).
> It probably doesn't make a difference, but we could also add a
> sha1_array_grow() for this case.
>
> I think it could be used in all three spots you converted in this patch.

We coulddo that, yes.  In the case above we have the number already, in 
the other cases we'd have to count.

But I don't think it's worth it here.  ALLOC_GROW gives us 24 entries 
initially, which should be enough in most cases -- I'm not sure I want 
to see combined diff of that many tree.  And 24 times 20 bytes is small 
enough to not cause any memory allocation issues.

Taking out the counting and not having to worry about the number of 
items is a feature in my book.  Simple timings before and after the 
patch didn't show any significant difference.

René