From: Kirill Smelkov <kirr@navytux.spb.ru>
To: Junio C Hamano <gitster@pobox.com>
Cc: Erik Faye-Lund <kusmabite@gmail.com>,
GIT Mailing-list <git@vger.kernel.org>,
Brandon Casey <drafnel@gmail.com>,
Marius Storm-Olsen <mstormo@gmail.com>,
Johannes Sixt <j6t@kdbg.org>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Ramsay Jones <ramsay@ramsay1.demon.co.uk>,
Gerrit Pape <pape@smarden.org>,
Petr Salinger <Petr.Salinger@seznam.cz>,
Jonathan Nieder <jrnieder@gmail.com>,
Thomas Schwinge <tschwinge@gnu.org>,
kirr@mns.spb.ru
Subject: Re: [PATCH 17/19] Portable alloca for Git
Date: Thu, 27 Mar 2014 18:22:50 +0400 [thread overview]
Message-ID: <20140327142250.GC17333@mini.zxlink> (raw)
In-Reply-To: <xmqq1txrp8ur.fsf@gitster.dls.corp.google.com>
On Mon, Mar 24, 2014 at 02:47:24PM -0700, Junio C Hamano wrote:
> Kirill Smelkov <kirr@mns.spb.ru> writes:
>
> > On Fri, Feb 28, 2014 at 06:19:58PM +0100, Erik Faye-Lund wrote:
> >> On Fri, Feb 28, 2014 at 6:00 PM, Kirill Smelkov <kirr@mns.spb.ru> wrote:
> >> ...
> >> > In fact that would be maybe preferred, for maintainers to enable alloca
> >> > with knowledge and testing, as one person can't have them all at hand.
> >>
> >> Yeah, you're probably right.
> >
> > Erik, the patch has been merged into pu today. Would you please
> > follow-up with tested MINGW change?
>
> Sooo.... I lost track but this discussion seems to have petered out
> around here. I think the copy we have had for a while on 'pu' is
> basically sound, and can easily built on by platform folks by adding
> or removing the -DHAVE_ALLOCA_H from the Makefile.
Yes, that is all correct - that version works and we can improve it in
the future with platform-specific follow-up patches, if needed.
Please pick up the patch with ack from Thomas Schwinge.
Thanks,
Kirill
(please keep author email)
---- 8< ----
From: Kirill Smelkov <kirr@mns.spb.ru>
Date: Mon, 24 Feb 2014 20:21:49 +0400
Subject: [PATCH v1a] Portable alloca for Git
In the next patch we'll have to use alloca() for performance reasons,
but since alloca is non-standardized and is not portable, let's have a
trick with compatibility wrappers:
1. at configure time, determine, do we have working alloca() through
alloca.h, and define
#define HAVE_ALLOCA_H
if yes.
2. in code
#ifdef HAVE_ALLOCA_H
# include <alloca.h>
# 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.
Both autoconf and config.mak.uname configurations were updated. For
autoconf, we are not bothering considering cases, when no alloca.h is
available, but alloca() works some other way - its simply alloca.h is
available and works or not, everything else is deep legacy.
For config.mak.uname, I've tried to make my almost-sure guess for where
alloca() is available, but since I only have access to Linux it is the
only change I can be sure about myself, with relevant to other changed
systems people Cc'ed.
NOTE
SunOS and Windows had explicit -DHAVE_ALLOCA_H in their configurations.
I've changed that to now-common HAVE_ALLOCA_H=YesPlease which should be
correct.
Cc: Brandon Casey <drafnel@gmail.com>
Cc: Marius Storm-Olsen <mstormo@gmail.com>
Cc: Johannes Sixt <j6t@kdbg.org>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Cc: Gerrit Pape <pape@smarden.org>
Cc: Petr Salinger <Petr.Salinger@seznam.cz>
Cc: Jonathan Nieder <jrnieder@gmail.com>
Cc: Thomas Schwinge <tschwinge@gnu.org>
Acked-by: Thomas Schwinge <thomas@codesourcery.com> (GNU Hurd changes)
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Changes since v1:
- added ack for GNU/Hurd.
Makefile | 6 ++++++
config.mak.uname | 10 ++++++++--
configure.ac | 8 ++++++++
git-compat-util.h | 8 ++++++++
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index dddaf4f..0334806 100644
--- a/Makefile
+++ b/Makefile
@@ -30,6 +30,8 @@ all::
# Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
# /foo/bar/include and /foo/bar/lib directories.
#
+# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header.
+#
# Define NO_CURL if you do not have libcurl installed. git-http-fetch and
# git-http-push are not built, and you cannot use http:// and https://
# transports (neither smart nor dumb).
@@ -1099,6 +1101,10 @@ ifdef USE_LIBPCRE
EXTLIBS += -lpcre
endif
+ifdef HAVE_ALLOCA_H
+ BASIC_CFLAGS += -DHAVE_ALLOCA_H
+endif
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..71602ee 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -28,6 +28,7 @@ ifeq ($(uname_S),OSF1)
NO_NSEC = YesPlease
endif
ifeq ($(uname_S),Linux)
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -35,6 +36,7 @@ ifeq ($(uname_S),Linux)
HAVE_DEV_TTY = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -103,6 +105,7 @@ ifeq ($(uname_S),SunOS)
NEEDS_NSL = YesPlease
SHELL_PATH = /bin/bash
SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
+ HAVE_ALLOCA_H = YesPlease
NO_STRCASESTR = YesPlease
NO_MEMMEM = YesPlease
NO_MKDTEMP = YesPlease
@@ -146,7 +149,7 @@ ifeq ($(uname_S),SunOS)
endif
INSTALL = /usr/ucb/install
TAR = gtar
- BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__ -DHAVE_ALLOCA_H
+ BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__
endif
ifeq ($(uname_O),Cygwin)
ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
@@ -166,6 +169,7 @@ ifeq ($(uname_O),Cygwin)
else
NO_REGEX = UnfortunatelyYes
endif
+ HAVE_ALLOCA_H = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
@@ -239,6 +243,7 @@ ifeq ($(uname_S),AIX)
endif
ifeq ($(uname_S),GNU)
# GNU/Hurd
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -316,6 +321,7 @@ endif
ifeq ($(uname_S),Windows)
GIT_VERSION := $(GIT_VERSION).MSVC
pathsep = ;
+ HAVE_ALLOCA_H = YesPlease
NO_PREAD = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
NO_LIBGEN_H = YesPlease
@@ -363,7 +369,7 @@ ifeq ($(uname_S),Windows)
COMPAT_OBJS = compat/msvc.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
compat/win32/dirent.o
- COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
+ COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
PTHREAD_LIBS =
diff --git a/configure.ac b/configure.ac
index 2f43393..0eae704 100644
--- a/configure.ac
+++ b/configure.ac
@@ -272,6 +272,14 @@ AS_HELP_STRING([], [ARG can be also prefix for libpcre library and hea
GIT_CONF_SUBST([LIBPCREDIR])
fi)
#
+# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header.
+AC_FUNC_ALLOCA
+case $ac_cv_working_alloca_h in
+ yes) HAVE_ALLOCA_H=YesPlease;;
+ *) HAVE_ALLOCA_H='';;
+esac
+GIT_CONF_SUBST([HAVE_ALLOCA_H])
+#
# Define NO_CURL if you do not have curl installed. git-http-pull and
# git-http-push are not built, and you cannot use http:// and https://
# transports.
diff --git a/git-compat-util.h b/git-compat-util.h
index cbd86c3..63b2b3b 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -526,6 +526,14 @@ extern void release_pack_memory(size_t);
typedef void (*try_to_free_t)(size_t);
extern try_to_free_t set_try_to_free_routine(try_to_free_t);
+#ifdef HAVE_ALLOCA_H
+# include <alloca.h>
+# 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
extern char *xstrdup(const char *str);
extern void *xmalloc(size_t size);
extern void *xmallocz(size_t size);
--
1.9.rc0.143.g6fd479e
next prev parent reply other threads:[~2014-03-27 14:19 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-24 16:21 [PATCH v2 00/19] Multiparent diff tree-walker + combine-diff speedup Kirill Smelkov
2014-02-24 16:21 ` [PATCH 01/19] combine-diff: move show_log_first logic/action out of paths scanning Kirill Smelkov
2014-02-24 16:21 ` [PATCH 02/19] combine-diff: move changed-paths scanning logic into its own function Kirill Smelkov
2014-02-24 16:21 ` [PATCH 03/19] tree-diff: no need to manually verify that there is no mode change for a path Kirill Smelkov
2014-02-24 16:21 ` [PATCH 04/19] tree-diff: no need to pass match to skip_uninteresting() Kirill Smelkov
2014-02-24 16:21 ` [PATCH 05/19] tree-diff: show_tree() is not needed Kirill Smelkov
2014-02-24 16:21 ` [PATCH 06/19] tree-diff: consolidate code for emitting diffs and recursion in one place Kirill Smelkov
2014-02-24 16:21 ` [PATCH 07/19] tree-diff: don't assume compare_tree_entry() returns -1,0,1 Kirill Smelkov
2014-02-24 16:21 ` [PATCH 08/19] tree-diff: move all action-taking code out of compare_tree_entry() Kirill Smelkov
2014-02-24 16:21 ` [PATCH 09/19] tree-diff: rename compare_tree_entry -> tree_entry_pathcmp Kirill Smelkov
2014-02-24 16:21 ` [PATCH 10/19] tree-diff: show_path prototype is not needed anymore Kirill Smelkov
2014-02-24 16:21 ` [PATCH 11/19] tree-diff: simplify tree_entry_pathcmp Kirill Smelkov
2014-03-24 21:25 ` Junio C Hamano
2014-03-25 9:23 ` Kirill Smelkov
2014-02-24 16:21 ` [PATCH 12/19] tree-diff: remove special-case diff-emitting code for empty-tree cases Kirill Smelkov
2014-03-24 21:18 ` Junio C Hamano
2014-03-25 9:20 ` Kirill Smelkov
2014-03-25 17:45 ` Junio C Hamano
2014-03-26 18:32 ` Kirill Smelkov
2014-03-25 22:07 ` Junio C Hamano
2014-02-24 16:21 ` [PATCH 13/19] tree-diff: diff_tree() should now be static Kirill Smelkov
2014-02-24 16:21 ` [PATCH v2 14/19] tree-diff: rework diff_tree interface to be sha1 based Kirill Smelkov
2014-03-24 21:36 ` Junio C Hamano
2014-03-25 9:22 ` Kirill Smelkov
2014-03-25 17:46 ` Junio C Hamano
2014-03-26 19:52 ` Kirill Smelkov
2014-03-26 21:34 ` Junio C Hamano
2014-03-27 14:24 ` Kirill Smelkov
2014-03-27 18:48 ` Junio C Hamano
2014-03-27 19:43 ` Kirill Smelkov
2014-03-28 6:52 ` Johannes Sixt
2014-03-28 17:06 ` Junio C Hamano
2014-03-28 17:46 ` Johannes Sixt
2014-03-28 18:36 ` Junio C Hamano
2014-03-28 19:08 ` Johannes Sixt
2014-03-28 19:27 ` Junio C Hamano
2014-02-24 16:21 ` [PATCH 15/19] tree-diff: no need to call "full" diff_tree_sha1 from show_path() Kirill Smelkov
2014-03-27 14:21 ` Kirill Smelkov
2014-02-24 16:21 ` [PATCH v2 16/19] tree-diff: reuse base str(buf) memory on sub-tree recursion Kirill Smelkov
2014-03-24 21:43 ` Junio C Hamano
2014-03-25 9:23 ` Kirill Smelkov
2014-03-27 14:22 ` Kirill Smelkov
2014-02-24 16:21 ` [PATCH 17/19] Portable alloca for Git Kirill Smelkov
2014-02-28 10:58 ` Thomas Schwinge
2014-02-28 13:44 ` Erik Faye-Lund
2014-02-28 13:50 ` Erik Faye-Lund
2014-02-28 17:00 ` Kirill Smelkov
2014-02-28 17:19 ` Erik Faye-Lund
2014-03-05 9:31 ` Kirill Smelkov
2014-03-24 21:47 ` Junio C Hamano
2014-03-27 14:22 ` Kirill Smelkov [this message]
2014-04-09 12:48 ` Kirill Smelkov
2014-04-09 13:01 ` Erik Faye-Lund
2014-04-10 17:30 ` Junio C Hamano
2014-02-24 16:21 ` [PATCH v2 18/19] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well Kirill Smelkov
2014-03-27 14:23 ` Kirill Smelkov
2014-04-04 18:42 ` Junio C Hamano
2014-04-06 21:46 ` Kirill Smelkov
2014-04-07 17:29 ` Junio C Hamano
2014-04-07 20:26 ` Kirill Smelkov
2014-04-07 18:07 ` Junio C Hamano
2014-02-24 16:21 ` [PATCH 19/19] combine-diff: speed it up, by using multiparent diff tree-walker directly Kirill Smelkov
2014-02-24 23:43 ` [PATCH v2 00/19] Multiparent diff tree-walker + combine-diff speedup Duy Nguyen
2014-02-25 10:38 ` 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=20140327142250.GC17333@mini.zxlink \
--to=kirr@navytux.spb.ru \
--cc=Johannes.Schindelin@gmx.de \
--cc=Petr.Salinger@seznam.cz \
--cc=drafnel@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jrnieder@gmail.com \
--cc=kirr@mns.spb.ru \
--cc=kusmabite@gmail.com \
--cc=mstormo@gmail.com \
--cc=pape@smarden.org \
--cc=ramsay@ramsay1.demon.co.uk \
--cc=tschwinge@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).