Re: [PATCH] Optimize common pattern of alloc_ref from string
On Sun, May 11, 2008 at 11:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> But really my main worry is that now we have _two_ functions which
>> allocate refs, so if "struct ref" ever grows a new field that needs
>> initializing, it has to go in two places (whereas if alloc_ref_from_str
>> calls alloc_ref, it works automatically).
>
> This is a very good point. We really do not want a micro-optimization in
> a way that hurts maintainability.
>
> Krzysztof's patch has the new function with the duplicated allocation
> implementation between the base allocator function and the existing
> copy_ref() function which also has yet another duplicated allocation
> implementation. When somebody needs to modify one, it is likely to be
> noticed that these three go hand-in-hand, hopefully ;-).
>
> By the way, why isn't alloc_ref() doing xcalloc(), I have to wonder...
I've sent updated patch that incorporates the feedback. Changes wrt.
to this patch:
* simpler version of alloc_ref_from_str() per Jeff's suggestion
* switches alloc_ref() to xcalloc() per Junio's suggestion
* uses alloc_ref_from_str() in one more place
* better (?) commit message
-- kjk
Krzysztof Kowalczyk <kkowalczyk@gmail.com>