From: Junio C Hamano <gitster@pobox•com>
To: Kirill Smelkov <kirr@navytux•spb.ru>
Cc: kirr@mns•spb.ru, git@vger•kernel.org
Subject: Re: [PATCH 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
Date: Tue, 18 Feb 2014 13:29:30 -0800 [thread overview]
Message-ID: <xmqqtxbwcdol.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140216080829.GA3820@mini.zxlink> (Kirill Smelkov's message of "Sun, 16 Feb 2014 12:08:29 +0400")
Kirill Smelkov <kirr@navytux•spb.ru> 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.
next prev parent reply other threads:[~2014-02-18 21:29 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-13 14:02 [PATCH 0/2] Multiparent diff tree-walker + combine-diff speedup Kirill Smelkov
2014-02-13 14:02 ` [PATCH 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well Kirill Smelkov
2014-02-13 19:51 ` Junio C Hamano
2014-02-14 12:15 ` Kirill Smelkov
2014-02-14 17:37 ` Junio C Hamano
2014-02-16 8:08 ` Kirill Smelkov
2014-02-18 21:29 ` Junio C Hamano [this message]
2014-02-13 14:02 ` [PATCH 2/2] combine-diff: speed it up, by using multiparent diff tree-walker directly Kirill Smelkov
2014-02-13 19:55 ` Junio C Hamano
2014-02-14 12:16 ` Kirill Smelkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqtxbwcdol.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=kirr@mns$(echo .)spb.ru \
--cc=kirr@navytux$(echo .)spb.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox