Re: [PATCH 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 18, 2014, 21:29 UTC
- Message-ID
- <xmqqtxbwcdol.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20140216080829.GA3820@mini.zxlink>
Kirill Smelkov <kirr@navytux.spb.ru> writes:
Show 51 quoted lines
>> > 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.