Subject: Re: [PATCH 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well

From: Junio C Hamano <[email-masked]>
Date: Tue, 18 Feb 2014 13:29:30 -0800

Kirill Smelkov <[email-masked]> writes:

>> > 2) alloca(), for small arrays, is used for the same reason - if we change
>> > it to xmalloc()/free() the timings get worse
>> 
>> Do you see any use of it outside compat/?
>> 
>> I thought we specifically avoid alloca() for portability.  Also we
>> do not use variable-length-arrays on the stack either, I think.
>
> No, no usage outside compat/ and I knew alloca and VLAs are not used in
> Git codebase for portability, and I understand alloca will be
> criticized, but wanted to start the discussion rolling.
>
> I've actually started without alloca, and used xmalloc/free for
> [nparent] vectors, but the impact was measurable, so it just had to be
> changed to something more optimal.
>
> For me, personally, alloca is ok, but I understand there could be
> portability issues (by the way, what compiler/system Git cares about
> does not have working alloca?). Thats why I propose we do the following
>
> 1. at configure time, determine, do we have working alloca, and define
>
>     #define HAVE_ALLOCA
>
>    if yes.
>
> 2. in code
>
>     #ifdef HAVE_ALLOCA
>     # define xalloca(size)      (alloca(size))
>     # define xalloca_free(p)    do {} while(0)
>     #else
>     # define xalloca(size)      (xmalloc(size))
>     # define xalloca_free(p)    (free(p))
>     #endif
>
>    and use it like
>
>    func() {
>        p = xalloca(size);
>        ...
>
>        xalloca_free(p);
>    }
>
> This way, for systems, where alloca is available, we'll have optimal
> on-stack allocations with fast executions. On the other hand, on
> systems, where alloca is not available, this gracefully fallbacks to
> xmalloc/free.
>
> Please tell me what you think.

I guess the above is clean enough, and we cannot do better than that,
if we want to use alloca() on platforms where we can.