* Re: [PATCH 0/4] Fix various integer overflows
From: Junio C Hamano @ 2010-01-26 19:58 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>
Looks trivially correct; thanks.
^ permalink raw reply
* Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
From: Junio C Hamano @ 2010-01-26 19:58 UTC (permalink / raw)
To: Jeff King; +Cc: git, Johannes Schindelin
In-Reply-To: <20100126130745.GB28179@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The first one is that @{usptream} silently becomes @{0}. I think
> we need to double-check whether approxidate found absolutely nothing,
> and complain if that is the case.
This is not a new problem introduced by Dscho's @{u} series; it was there
even before 861f00e (fix reflog approxidate parsing bug, 2008-04-30).
Nevertheless, it would be nice to fix it.
-- >8 --
Subject: approxidate_careful() reports errorneous date string
For a long time, the time based reflog syntax (e.g. master@{yesterday})
didn't complain when the "human readable" timestamp was misspelled, as
the underlying mechanism tried to be as lenient as possible. The funny
thing was that parsing of "@{now}" even relied on the fact that anything
not recognized by the machinery returned the current timestamp.
Introduce approxidate_careful() that takes an optional pointer to an
integer, that gets assigned 1 when the input does not make sense as a
timestamp.
As I am too lazy to fix all the callers that use approxidate(), most of
the callers do not take advantage of the error checking, but convert the
code to parse reflog to use it as a demonstration.
Tests are mostly from Jeff King.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cache.h | 3 ++-
date.c | 43 +++++++++++++++++++++++++++++++++++--------
sha1_name.c | 5 ++++-
t/t0101-at-syntax.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/cache.h b/cache.h
index b3370eb..f0fea2d 100644
--- a/cache.h
+++ b/cache.h
@@ -762,7 +762,8 @@ const char *show_date_relative(unsigned long time, int tz,
size_t timebuf_size);
int parse_date(const char *date, char *buf, int bufsize);
void datestamp(char *buf, int bufsize);
-unsigned long approxidate(const char *);
+#define approxidate(s) approxidate_careful(s, NULL)
+unsigned long approxidate_careful(const char *, int *);
unsigned long approxidate_relative(const char *date, const struct timeval *now);
enum date_mode parse_date_format(const char *format);
diff --git a/date.c b/date.c
index 45f3684..002aa3c 100644
--- a/date.c
+++ b/date.c
@@ -696,6 +696,11 @@ static unsigned long update_tm(struct tm *tm, struct tm *now, unsigned long sec)
return n;
}
+static void date_now(struct tm *tm, struct tm *now, int *num)
+{
+ update_tm(tm, now, 0);
+}
+
static void date_yesterday(struct tm *tm, struct tm *now, int *num)
{
update_tm(tm, now, 24*60*60);
@@ -770,6 +775,7 @@ static const struct special {
{ "PM", date_pm },
{ "AM", date_am },
{ "never", date_never },
+ { "now", date_now },
{ NULL }
};
@@ -790,7 +796,7 @@ static const struct typelen {
{ NULL }
};
-static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm *now, int *num)
+static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm *now, int *num, int *touched)
{
const struct typelen *tl;
const struct special *s;
@@ -804,6 +810,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
int match = match_string(date, month_names[i]);
if (match >= 3) {
tm->tm_mon = i;
+ *touched = 1;
return end;
}
}
@@ -812,6 +819,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
int len = strlen(s->name);
if (match_string(date, s->name) == len) {
s->fn(tm, now, num);
+ *touched = 1;
return end;
}
}
@@ -821,11 +829,14 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
int len = strlen(number_name[i]);
if (match_string(date, number_name[i]) == len) {
*num = i;
+ *touched = 1;
return end;
}
}
- if (match_string(date, "last") == 4)
+ if (match_string(date, "last") == 4) {
*num = 1;
+ *touched = 1;
+ }
return end;
}
@@ -835,6 +846,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
if (match_string(date, tl->type) >= len-1) {
update_tm(tm, now, tl->length * *num);
*num = 0;
+ *touched = 1;
return end;
}
tl++;
@@ -852,6 +864,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
diff += 7*n;
update_tm(tm, now, diff * 24 * 60 * 60);
+ *touched = 1;
return end;
}
}
@@ -866,6 +879,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
tm->tm_year--;
}
tm->tm_mon = n;
+ *touched = 1;
return end;
}
@@ -873,6 +887,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
update_tm(tm, now, 0); /* fill in date fields if needed */
tm->tm_year -= *num;
*num = 0;
+ *touched = 1;
return end;
}
@@ -929,9 +944,12 @@ static void pending_number(struct tm *tm, int *num)
}
}
-static unsigned long approxidate_str(const char *date, const struct timeval *tv)
+static unsigned long approxidate_str(const char *date,
+ const struct timeval *tv,
+ int *error_ret)
{
int number = 0;
+ int touched = 0;
struct tm tm, now;
time_t time_sec;
@@ -951,33 +969,42 @@ static unsigned long approxidate_str(const char *date, const struct timeval *tv)
if (isdigit(c)) {
pending_number(&tm, &number);
date = approxidate_digit(date-1, &tm, &number);
+ touched = 1;
continue;
}
if (isalpha(c))
- date = approxidate_alpha(date-1, &tm, &now, &number);
+ date = approxidate_alpha(date-1, &tm, &now, &number, &touched);
}
pending_number(&tm, &number);
+ if (!touched)
+ *error_ret = 1;
return update_tm(&tm, &now, 0);
}
unsigned long approxidate_relative(const char *date, const struct timeval *tv)
{
char buffer[50];
+ int errors = 0;
if (parse_date(date, buffer, sizeof(buffer)) > 0)
return strtoul(buffer, NULL, 0);
- return approxidate_str(date, tv);
+ return approxidate_str(date, tv, &errors);
}
-unsigned long approxidate(const char *date)
+unsigned long approxidate_careful(const char *date, int *error_ret)
{
struct timeval tv;
char buffer[50];
+ int dummy = 0;
+ if (!error_ret)
+ error_ret = &dummy;
- if (parse_date(date, buffer, sizeof(buffer)) > 0)
+ if (parse_date(date, buffer, sizeof(buffer)) > 0) {
+ *error_ret = 0;
return strtoul(buffer, NULL, 0);
+ }
gettimeofday(&tv, NULL);
- return approxidate_str(date, &tv);
+ return approxidate_str(date, &tv, error_ret);
}
diff --git a/sha1_name.c b/sha1_name.c
index 9215ad1..ed4c028 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -413,8 +413,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
} else if (0 <= nth)
at_time = 0;
else {
+ int errors = 0;
char *tmp = xstrndup(str + at + 2, reflog_len);
- at_time = approxidate(tmp);
+ at_time = approxidate_careful(tmp, &errors);
+ if (errors)
+ die("Bogus timestamp '%s'", tmp);
free(tmp);
}
if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
diff --git a/t/t0101-at-syntax.sh b/t/t0101-at-syntax.sh
new file mode 100755
index 0000000..ccabc37
--- /dev/null
+++ b/t/t0101-at-syntax.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+test_description='various @{whatever} syntax tests'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit one &&
+ test_commit two
+'
+
+check_at() {
+ echo "$2" >expect &&
+ git log -1 --format=%s "$1" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success '@{0} shows current' '
+ check_at @{0} two
+'
+
+test_expect_success '@{1} shows old' '
+ check_at @{1} one
+'
+
+test_expect_success '@{now} shows current' '
+ check_at @{now} two
+'
+
+test_expect_success '@{30.years.ago} shows old' '
+ check_at @{30.years.ago} one
+'
+
+test_expect_success 'silly approxidates work' '
+ check_at @{3.hot.dogs.and.30.years.ago} one
+'
+
+test_expect_success 'notice misspelled upstream' '
+ test_must_fail git log -1 --format=%s @{usptream}
+'
+
+test_expect_success 'complain about total nonsense' '
+ test_must_fail git log -1 --format=%s @{utter.bogosity}
+'
+
+test_done
^ permalink raw reply related
* [PATCH 2/2] Allow use of []-wrapped addresses in git://
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
In-Reply-To: <1264530282-4783-1-git-send-email-ilari.liusvaara@elisanet.fi>
Allow using "["<host>"]":<port> and "["<host>"]" notations in git://
host addresses. This is needed to be able to connect to addresses
that contain ':' (e.g. numeric IPv6 addresses). Also send the host
header []-wrapped so it can actually be parsed by remote end.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
connect.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/connect.c b/connect.c
index 3a12562..20054e4 100644
--- a/connect.c
+++ b/connect.c
@@ -502,12 +502,18 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
c = ':';
}
+ /*
+ * Don't do destructive transforms with git:// as that
+ * protocol code does '[]' dewrapping of its own.
+ */
if (host[0] == '[') {
end = strchr(host + 1, ']');
if (end) {
- *end = 0;
+ if (protocol != PROTO_GIT) {
+ *end = 0;
+ host++;
+ }
end++;
- host++;
} else
end = host;
} else
--
1.6.6.1.439.gf06b6
^ permalink raw reply related
* [PATCH 1/2] Support addresses with ':' in git-daemon
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
In-Reply-To: <1264530282-4783-1-git-send-email-ilari.liusvaara@elisanet.fi>
If host address could have ':' in it (e.g. numeric IPv6 address), then
host and port could not be uniquely parsed. Fix this by parsing the
"["<host>"]":<port> and "["<host>"]" notations. Currently the built-in
git:// client would send <host>:<port> or <host> for such thing, but
it doesn't matter as due to bugs, resolving address fails if <host>
contains ':'.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
daemon.c | 34 ++++++++++++++++++++++++++++++----
1 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/daemon.c b/daemon.c
index 360635e..6c2bd97 100644
--- a/daemon.c
+++ b/daemon.c
@@ -399,6 +399,33 @@ static char *xstrdup_tolower(const char *str)
return dup;
}
+static void parse_host_and_port(char *hostport, char **host,
+ char **port)
+{
+ if (*hostport == '[') {
+ char *end;
+
+ end = strchr(hostport, ']');
+ if (!end)
+ die("Invalid reqeuest ('[' without ']')");
+ *end = '\0';
+ *host = hostport + 1;
+ if (!end[1])
+ *port = NULL;
+ else if (end[1] == ':')
+ *port = end + 2;
+ else
+ die("Garbage after end of host part");
+ } else {
+ *host = hostport;
+ *port = strrchr(hostport, ':');
+ if (*port) {
+ *port = '\0';
+ ++*port;
+ }
+ }
+}
+
/*
* Read the host as supplied by the client connection.
*/
@@ -415,11 +442,10 @@ static void parse_host_arg(char *extra_args, int buflen)
vallen = strlen(val) + 1;
if (*val) {
/* Split <host>:<port> at colon. */
- char *host = val;
- char *port = strrchr(host, ':');
+ char *host;
+ char *port;
+ parse_host_and_port(val, &host, &port);
if (port) {
- *port = 0;
- port++;
free(tcp_port);
tcp_port = xstrdup(port);
}
--
1.6.6.1.439.gf06b6
^ permalink raw reply related
* [PATCH 0/2] Allow using ':' in git:// hostname.
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
This series fixes two problems with using addresses containing ':'
(e.g. IPv6 numeric addresses) with git://:
1) ':' in hostname makes vhost headers impossible to parse
If there is ':' in address, the vhost headers become impossible
to parse because ':' is also splits host and port and port is
optional. Change git-daemon to be able to perform address unwrapping
so there is uniquely parseable syntax for hostnames containg
':' (this is compatible to how git-remote-gits encodes such vhost
headers and how git-daemon2[1] decodes them).
2) Client double-unwraps addresses
With git://, the addresses are unwrapped twice, which breaks
address parsing for addresses enclosed by [], which in turn is
required for hostnames containing ':'. This is changed to unwarp
the addresses only once. This also changes wrapped addresses to
be sent as wrapped for vhost headers (the first patch adds ability
to parse this).
[1] The reference implementation of gits:// server daemon.
Ilari Liusvaara (2):
Support addresses with ':' in git-daemon
Allow use of []-wrapped addresses in git://
connect.c | 10 ++++++++--
daemon.c | 34 ++++++++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCH 4/4] Fix integer overflow in unpack_compressed_entry()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
sha1_file.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 39f0844..ea2ea75 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1517,8 +1517,7 @@ static void *unpack_compressed_entry(struct packed_git *p,
z_stream stream;
unsigned char *buffer, *in;
- buffer = xmalloc(size + 1);
- buffer[size] = 0;
+ buffer = xmallocz(size);
memset(&stream, 0, sizeof(stream));
stream.next_out = buffer;
stream.avail_out = size + 1;
--
1.6.6.1.439.gf06b6
^ permalink raw reply related
* [PATCH 1/4] Add xmallocz()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>
Add routine for allocating NUL-terminated memory block without risking
integer overflow in addition of +1 for NUL byte.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
git-compat-util.h | 1 +
wrapper.c | 12 +++++++++++-
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 620a7c6..a3c4537 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -348,6 +348,7 @@ extern void release_pack_memory(size_t, int);
extern char *xstrdup(const char *str);
extern void *xmalloc(size_t size);
+extern void *xmallocz(size_t size);
extern void *xmemdupz(const void *data, size_t len);
extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
diff --git a/wrapper.c b/wrapper.c
index c9be140..dd7b6ee 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -34,6 +34,16 @@ void *xmalloc(size_t size)
return ret;
}
+void *xmallocz(size_t size)
+{
+ void *ret;
+ if (size + 1 < size)
+ die("Data too large to fit into virtual memory space.");
+ ret = xmalloc(size + 1);
+ ((char*)ret)[size] = 0;
+ return ret;
+}
+
/*
* xmemdupz() allocates (len + 1) bytes of memory, duplicates "len" bytes of
* "data" to the allocated memory, zero terminates the allocated memory,
@@ -42,7 +52,7 @@ void *xmalloc(size_t size)
*/
void *xmemdupz(const void *data, size_t len)
{
- char *p = xmalloc(len + 1);
+ char *p = xmallocz(len);
memcpy(p, data, len);
p[len] = '\0';
return p;
--
1.6.6.1.439.gf06b6
^ permalink raw reply related
* [PATCH 2/4] Fix integer overflow in patch_delta()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
patch-delta.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/patch-delta.c b/patch-delta.c
index e02e13b..d218faa 100644
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -33,8 +33,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
/* now the result size */
size = get_delta_hdr_size(&data, top);
- dst_buf = xmalloc(size + 1);
- dst_buf[size] = 0;
+ dst_buf = xmallocz(size);
out = dst_buf;
while (data < top) {
--
1.6.6.1.439.gf06b6
^ permalink raw reply related
* [PATCH 3/4] Fix integer overflow in unpack_sha1_rest()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
sha1_file.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 12478a3..39f0844 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1166,7 +1166,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, const unsigned char *sha1)
{
int bytes = strlen(buffer) + 1;
- unsigned char *buf = xmalloc(1+size);
+ unsigned char *buf = xmallocz(size);
unsigned long n;
int status = Z_OK;
--
1.6.6.1.439.gf06b6
^ permalink raw reply related
* [PATCH 0/4] Fix various integer overflows
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
To: git
Fix integer overflows in patch_delta(), unpack_sha1_rest() and
unpack_compressed_entry().
These at least can cause git to segfault, possibly worse. Operations
that cause integer overflow are not possible to do (even whole virtual
memory space would not be sufficient), so die() instead.
Ilari Liusvaara (4):
Add xmallocz()
Fix integer overflow in patch_delta()
Fix integer overflow in unpack_sha1_rest()
Fix integer overflow in unpack_compressed_entry()
git-compat-util.h | 1 +
patch-delta.c | 3 +--
sha1_file.c | 5 ++---
wrapper.c | 12 +++++++++++-
4 files changed, 15 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCH] grep: use REG_STARTEND (if available) to speed up regexec
From: Benjamin Kramer @ 2010-01-26 17:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Fredrik Kuivinen, Git Mailing List, Linus Torvalds, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260846330.3574@localhost.localdomain>
BSD and glibc have an extension to regexec which takes a buffer + length pair
instead of a NUL-terminated string. Since we already have the length computed
this can save us a strlen call inside regexec.
Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
---
Resend of my previous patch with SOB and correct title.
grep.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/grep.c b/grep.c
index 8e1f7de..452c2cb 100644
--- a/grep.c
+++ b/grep.c
@@ -640,8 +640,15 @@ static int look_ahead(struct grep_opt *opt,
if (p->fixed)
hit = !fixmatch(p->pattern, bol, p->ignore_case, &m);
- else
+ else {
+#ifdef REG_STARTEND
+ m.rm_so = 0;
+ m.rm_eo = *left_p;
+ hit = !regexec(&p->regexp, bol, 1, &m, REG_STARTEND);
+#else
hit = !regexec(&p->regexp, bol, 1, &m, 0);
+#endif
+ }
if (!hit || m.rm_so < 0 || m.rm_eo < 0)
continue;
if (earliest < 0 || m.rm_so < earliest)
--
1.7.0.rc0.12.gc33c3
^ permalink raw reply related
* Re: [PATCH v4] Threaded grep
From: Junio C Hamano @ 2010-01-26 17:21 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git, Linus Torvalds, Johannes Sixt
In-Reply-To: <20100126114303.GA1854@fredrik-laptop>
Fredrik Kuivinen <frekui@gmail.com> writes:
> I just noticed that I forgot to take the read_sha1 lock in
> grep_tree. The result is a race condition in read_sha1_file.
>
> Here is a patch to fix this. It applies on top of ae35c68 (Threaded
> grep). Could you please squash it in?
Thanks; will do.
^ permalink raw reply
* Re: [PATCH v4] Threaded grep
From: Mike Hommey @ 2010-01-26 17:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Benjamin Kramer, Fredrik Kuivinen, Git Mailing List,
Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260846330.3574@localhost.localdomain>
On Tue, Jan 26, 2010 at 08:56:50AM -0800, Linus Torvalds wrote:
<snip>
> although it is worth noting that that machine has an Intel SSD, which is
> why it gets sped up so much by parallel IO (there's no seek penalty, and
> it is able to read multiple channels in parallel, so this gives much
> better IO patterns for it - with rotational media the numbers might be
> very different).
For rotational disks, using FIEMAP to get the position of the files on
disk to reorder how we read them could help.
Mike
^ permalink raw reply
* Re: [PATCH v4] Threaded grep
From: Linus Torvalds @ 2010-01-26 16:56 UTC (permalink / raw)
To: Benjamin Kramer
Cc: Fredrik Kuivinen, Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260836520.3574@localhost.localdomain>
On Tue, 26 Jan 2010, Linus Torvalds wrote:
>
> Goodie. It's noticeable for me. This is what I reported earlier:
>
> > > $ /usr/bin/time git grep void
> >
> > Before:
> >
> > real 0m1.144s
> >
> > After:
> > real 0m0.290s
>
> and with your patch I get
>
> real 0m0.239s
Btw, I have to also say that this whole performance reduction _feels_
good. It's very noticeable in normal use. "git grep" was always fast (it's
been getting a bit slower as the kernel has grown, though), but it used to
be still a noticeable pause.
Now it just -feels- very immediate. That quarter second is short enough
that I can see the pause, but I don't feel it. It's like the results just
"are there" rather than get searched for.
But perhaps even more importantly, it's also noticeable for me in the
cold-cache case. IOW, after I do
echo 3 > /proc/sys/vm/drop_caches
the threaded grep is able to do much better at reading the disk:
Before threading:
[torvalds@nehalem linux]$ time git grep void > /dev/null
real 0m11.745s
user 0m2.380s
sys 0m1.200s
After:
[torvalds@nehalem linux]$ time ~/git/git grep void > /dev/null
real 0m3.710s
user 0m2.564s
sys 0m2.076s
although it is worth noting that that machine has an Intel SSD, which is
why it gets sped up so much by parallel IO (there's no seek penalty, and
it is able to read multiple channels in parallel, so this gives much
better IO patterns for it - with rotational media the numbers might be
very different).
IOW, the whole threaded grep thing is a 4x performance improvement in
hot-cache, and a 3x improvement in cold-cache.
Major good mojo.
Linus
^ permalink raw reply
* Re: [PATCH v4] Threaded grep
From: Linus Torvalds @ 2010-01-26 16:44 UTC (permalink / raw)
To: Benjamin Kramer
Cc: Fredrik Kuivinen, Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <4B5F1894.4070509@googlemail.com>
On Tue, 26 Jan 2010, Benjamin Kramer wrote:
>
> BSD and glibc have an "REG_STARTEND" flag to do that. I made a small
> PoC patch to use it if it's available but it didn't give any significant
> speedup on my system.
Goodie. It's noticeable for me. This is what I reported earlier:
> > $ /usr/bin/time git grep void
>
> Before:
>
> real 0m1.144s
> user 0m0.988s
> sys 0m0.148s
>
> After:
> real 0m0.290s
> user 0m1.732s
> sys 0m0.232s
and with your patch I get
real 0m0.239s
user 0m1.392s
sys 0m0.276s
and the profile shows no strlen in it:
57.12% git libc-2.11.1.so [.] re_search_internal
5.59% git [kernel] [k] copy_user_generic_string
4.09% git [kernel] [k] _raw_spin_lock
2.57% git [kernel] [k] intel_pmu_enable_all
2.46% git [kernel] [k] __d_lookup
1.94% git libc-2.11.1.so [.] re_string_reconstruct
1.87% git [kernel] [k] kmem_cache_alloc
1.68% git libc-2.11.1.so [.] _int_free
1.53% git [kernel] [k] find_get_page
1.43% git [kernel] [k] update_curr
1.27% git libc-2.11.1.so [.] __GI___libc_malloc
1.17% git [kernel] [k] _atomic_dec_and_lock
1.00% git libc-2.11.1.so [.] __GI_memcpy
Side note: the tailing end of the profiles aren't very stable, probably
because the grep executes so quickly and in so many threads, so the
functions in the one-percent range will move up and down the list
depending on just exactly where we happened to get profile hits.
Similarly, the raw_spin_lock numbers vary.
But the big picture is stable, and that 57% number (and the nonlock
copy_user_generic_string) is consistent. And your patch definitely helped
both actual performance and is visible in the profile: re_search_internal
went from ~52% to ~57%.
So ack on that patch. Looks like a good thing to do, and with the #ifdef,
it looks like it should just automatically DTRT based on regexec
implementation.
Linus
^ permalink raw reply
* Re: [PATCH v4] Threaded grep
From: Benjamin Kramer @ 2010-01-26 16:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Fredrik Kuivinen, Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260728260.3574@localhost.localdomain>
BSD and glibc have an extension to regexec which takes a buffer + length pair
instead of a NUL-terminated string. Since we already have the length
computed this can save us a strlen call.
---
On 26.01.10 16:28, Linus Torvalds wrote:
> so it's sadly internal to regex. It would be nice if there was a
> non-string interface to regexec (ie a "buffer + length" instead of a
> NUL-terminated string).
BSD and glibc have an "REG_STARTEND" flag to do that. I made a small
PoC patch to use it if it's available but it didn't give any significant
speedup on my system.
grep.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/grep.c b/grep.c
index d281a02..60cce46 100644
--- a/grep.c
+++ b/grep.c
@@ -675,8 +675,15 @@ static int look_ahead(struct grep_opt *opt,
if (p->fixed)
hit = !fixmatch(p->pattern, bol, p->ignore_case, &m);
- else
+ else {
+#ifdef REG_STARTEND
+ m.rm_so = 0;
+ m.rm_eo = *left_p;
+ hit = !regexec(&p->regexp, bol, 1, &m, REG_STARTEND);
+#else
hit = !regexec(&p->regexp, bol, 1, &m, 0);
+#endif
+ }
if (!hit || m.rm_so < 0 || m.rm_eo < 0)
continue;
if (earliest < 0 || m.rm_so < earliest)
--
1.7.0.rc0.12.gc33c3
^ permalink raw reply related
* Re: [PATCH 00/12] Re: Makefile: add missing header dependency rules
From: Jonathan Nieder @ 2010-01-26 16:19 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Jonathan Nieder wrote:
> The remainder of this series is based on a merge of master and
> patches 1-5:
[...]
> Jonathan Nieder (12):
> Makefile: add missing header file dependencies
> Makefile: make sure test helpers are rebuilt when headers change
> Makefile: remove wt-status.h from LIB_H
> Makefile: clean up http-walker.o dependency rules
> Makefile: drop dependency on $(wildcard */*.h)
> Makefile: transport.o depends on branch.h now
> Makefile: rearrange dependency rules
> Makefile: disable default implicit rules
> Makefile: list generated object files in OBJECTS macro
> Makefile: lazily compute header dependencies
> Makefile: list standalone program object files in PROGRAM_OBJS
> Teach Makefile to check header dependencies
For those would like to avoid redoing the conflict resolution (and
who wouldn’t?), this series is also available in the git repository
at
git://repo.or.cz/git/jrn.git autodep
If you’re interested, please take a look around and try it out. I’d
be happy to hear about any bugs you find.
Thanks again,
Jonathan
^ permalink raw reply
* [PATCH 12/12] Teach Makefile to check header dependencies
From: Jonathan Nieder @ 2010-01-26 15:57 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Add a target to use the gcc-generated makefile snippets for
dependencies on header files to check the hard-coded dependencies.
With this patch applied, if any dependencies are missing, then
make clean
make COMPUTE_HEADER_DEPENDENCIES=YesPlease
make CHECK_HEADER_DEPENDENCIES=YesPlease
will produce an error message like the following:
CHECK fast-import.o
missing dependencies: exec_cmd.h
make: *** [fast-import.o] Error 1
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That’s the end of the series. Thanks for reading.
Makefile | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/Makefile b/Makefile
index ceaae1c..be357a8 100644
--- a/Makefile
+++ b/Makefile
@@ -221,6 +221,9 @@ all::
# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
# and you want to avoid rebuilding objects when an unrelated header file
# changes.
+#
+# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
+# dependency rules.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1088,6 +1091,14 @@ endif
-include config.mak.autogen
-include config.mak
+ifdef CHECK_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES =
+endif
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+endif
+
ifdef SANE_TOOL_PATH
SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
@@ -1683,9 +1694,7 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
-ASM_SRC := $(wildcard $(OBJECTS:o=S))
-ASM_OBJ := $(ASM_SRC:S=o)
-C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+dep_files := $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d)
ifdef COMPUTE_HEADER_DEPENDENCIES
dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS))))
@@ -1693,33 +1702,89 @@ $(dep_dirs):
mkdir -p $@
missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
-else
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+ifdef CHECK_HEADER_DEPENDENCIES
+$(error cannot compute header dependencies outside a normal build. \
+Please unset CHECK_HEADER_DEPENDENCIES and try again)
+endif
+endif
+
+ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef CHECK_HEADER_DEPENDENCIES
dep_dirs =
missing_dep_dirs =
+dep_args =
+endif
+endif
+
+ifdef CHECK_HEADER_DEPENDENCIES
+ifndef PRINT_HEADER_DEPENDENCIES
+missing_deps = $(filter-out $(notdir $^), \
+ $(notdir $(shell $(MAKE) -s $@ \
+ CHECK_HEADER_DEPENDENCIES=YesPlease \
+ USE_COMPUTED_HEADER_DEPENDENCIES=YesPlease \
+ PRINT_HEADER_DEPENDENCIES=YesPlease)))
+endif
endif
+ASM_SRC := $(wildcard $(OBJECTS:o=S))
+ASM_OBJ := $(ASM_SRC:S=o)
+C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+
.SUFFIXES:
+ifdef PRINT_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c FORCE
+ echo $^
+$(ASM_OBJ): %.o: %.S FORCE
+ echo $^
+
+ifndef CHECK_HEADER_DEPENDENCIES
+$(error cannot print header dependencies during a normal build. \
+Please set CHECK_HEADER_DEPENDENCIES and try again)
+endif
+endif
+
+ifndef PRINT_HEADER_DEPENDENCIES
+ifdef CHECK_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c $(dep_files) FORCE
+ @set -e; echo CHECK $@; \
+ missing_deps="$(missing_deps)"; \
+ if test "$$missing_deps"; \
+ then \
+ echo missing dependencies: $$missing_deps; \
+ false; \
+ fi
+$(ASM_OBJ): %.o: %.S $(dep_files) FORCE
+ @set -e; echo CHECK $@; \
+ missing_deps="$(missing_deps)"; \
+ if test "$$missing_deps"; \
+ then \
+ echo missing dependencies: $$missing_deps; \
+ false; \
+ fi
+endif
+endif
+
+ifndef CHECK_HEADER_DEPENDENCIES
$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS FORCE
- $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+endif
-ifdef COMPUTE_HEADER_DEPENDENCIES
+%.s: %.c GIT-CFLAGS FORCE
+ $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+
+ifdef USE_COMPUTED_HEADER_DEPENDENCIES
# Take advantage of gcc's on-the-fly dependency generation
# See <http://gcc.gnu.org/gcc-3.0/features.html>.
-dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
-ifneq ($(dep_files),)
-include $(dep_files)
+dep_files_present := $(wildcard $(dep_files))
+ifneq ($(dep_files_present),)
+include $(dep_files_present)
endif
-
-dep_file = $(dir $@)deps/$(notdir $@).d
-dep_args = -MF $(dep_file) -MMD -MP
else
-dep_args =
-
# Dependencies on header files, for platforms that do not support
# the gcc -MMD option.
#
--
1.6.6
^ permalink raw reply related
* [PATCH 07/12] Makefile: rearrange dependency rules
From: Jonathan Nieder @ 2010-01-26 15:49 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Put rules listing dependencies of compiled objects (.o files) on
header files (.h files) in one place, to make them easier to
compare and modify all at once.
Add a GIT_OBJS variable listing objects that depend on LIB_H,
for similar reasons.
No change in build-time behavior intended.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 49 +++++++++++++++++++++++++------------------------
1 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/Makefile b/Makefile
index 5678991..84ce137 100644
--- a/Makefile
+++ b/Makefile
@@ -1666,6 +1666,12 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
+ git.o http.o http-walker.o remote-curl.o \
+ $(patsubst git-%$X,%.o,$(PROGRAMS))
+XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
+ xdiff/xmerge.o xdiff/xpatience.o
+
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
@@ -1673,6 +1679,25 @@ git.o git.spec \
%.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(GIT_OBJS): $(LIB_H)
+builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
+builtin-bundle.o bundle.o transport.o: bundle.h
+builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
+builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
+builtin-send-pack.o transport.o: send-pack.h
+builtin-log.o builtin-shortlog.o: shortlog.h
+builtin-prune.o builtin-reflog.o reachable.o: reachable.h
+builtin-commit.o builtin-revert.o wt-status.o: wt-status.h
+builtin-tar-tree.o archive-tar.o: tar.h
+builtin-pack-objects.o: thread-utils.h
+http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
+http.o http-walker.o http-push.o remote-curl.o: http.h
+
+
+xdiff-interface.o $(XDIFF_OBJS): \
+ xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
+ xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+
exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1696,10 +1721,6 @@ git-imap-send$X: imap-send.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
-http.o http-walker.o http-push.o remote-curl.o: http.h
-
-http.o http-walker.o remote-curl.o: $(LIB_H)
-
git-http-fetch$X: revision.o http.o http-walker.o http-fetch.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL)
@@ -1717,29 +1738,9 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
-builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
-builtin-bundle.o bundle.o transport.o: bundle.h
-builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
-builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
-builtin-send-pack.o transport.o: send-pack.h
-builtin-log.o builtin-shortlog.o: shortlog.h
-builtin-prune.o builtin-reflog.o reachable.o: reachable.h
-builtin-commit.o builtin-revert.o wt-status.o: wt-status.h
-builtin-tar-tree.o archive-tar.o: tar.h
-builtin-pack-objects.o: thread-utils.h
-http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
-
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
-XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
- xdiff/xmerge.o xdiff/xpatience.o
-xdiff-interface.o $(XDIFF_OBJS): \
- xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
- xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
-
$(XDIFF_LIB): $(XDIFF_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS)
--
1.6.6
^ permalink raw reply related
* [PATCH 11/12] Makefile: list standalone program object files in PROGRAM_OBJS
From: Jonathan Nieder @ 2010-01-26 15:54 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Because of new commands like git-remote-http, the OBJECTS list
contains fictitious objects such as remote-http.o. Thus any
out-of-tree rules that require all $(OBJECTS) to be buildable
are broken. Add a list of real program objects to avoid this
problem.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Preparing for patch 12.
Makefile | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 2e75f05..ceaae1c 100644
--- a/Makefile
+++ b/Makefile
@@ -341,6 +341,7 @@ COMPAT_CFLAGS =
COMPAT_OBJS =
LIB_H =
LIB_OBJS =
+PROGRAM_OBJS =
PROGRAMS =
SCRIPT_PERL =
SCRIPT_PYTHON =
@@ -390,12 +391,15 @@ EXTRA_PROGRAMS =
# ... and all the rest that could be moved out of bindir to gitexecdir
PROGRAMS += $(EXTRA_PROGRAMS)
-PROGRAMS += git-fast-import$X
-PROGRAMS += git-imap-send$X
-PROGRAMS += git-shell$X
-PROGRAMS += git-show-index$X
-PROGRAMS += git-upload-pack$X
-PROGRAMS += git-http-backend$X
+
+PROGRAM_OBJS += fast-import.o
+PROGRAM_OBJS += imap-send.o
+PROGRAM_OBJS += shell.o
+PROGRAM_OBJS += show-index.o
+PROGRAM_OBJS += upload-pack.o
+PROGRAM_OBJS += http-backend.o
+
+PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
TEST_PROGRAMS_NEED_X += test-ctype
@@ -1139,10 +1143,12 @@ else
REMOTE_CURL_PRIMARY = git-remote-http$X
REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X
REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
+ PROGRAM_OBJS += http-fetch.o
PROGRAMS += $(REMOTE_CURL_NAMES) git-http-fetch$X
curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
ifeq "$(curl_check)" "070908"
ifndef NO_EXPAT
+ PROGRAM_OBJS += http-push.o
PROGRAMS += git-http-push$X
endif
endif
@@ -1163,6 +1169,7 @@ endif
EXTLIBS += -lz
ifndef NO_POSIX_ONLY_PROGRAMS
+ PROGRAM_OBJS += daemon.o
PROGRAMS += git-daemon$X
endif
ifndef NO_OPENSSL
@@ -1670,9 +1677,8 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
-GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
- git.o http.o http-walker.o remote-curl.o \
- $(patsubst git-%$X,%.o,$(PROGRAMS))
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
+ git.o http.o http-walker.o remote-curl.o
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
--
1.6.6
^ permalink raw reply related
* [PATCH 10/12] Makefile: lazily compute header dependencies
From: Jonathan Nieder @ 2010-01-26 15:52 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Use the gcc -MMD -MP -MF options to generate dependency rules as
a byproduct when building .o files if the
COMPUTE_HEADER_DEPENDENCIES variable is defined. That variable
is left undefined by default for now.
As each object file is built, write a makefile fragment
containing its dependencies in the deps/ subdirectory of its
containing directory. The deps/ directories should be generated
if they are missing at the start of each build. So let each
object file depend on $(missing_dep_dirs), which lists only the
directories of this kind that are missing to avoid needlessly
regenerating files when the directories' timestamps change.
gcc learned the -MMD -MP -MF options in version 3.0, so most gcc
users should have them by now.
The dependencies this option computes are more specific than the
rough estimates hard-coded in the Makefile, greatly speeding up
rebuilds when only a little-used header file has changed.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
.gitignore | 1 +
Makefile | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/.gitignore b/.gitignore
index 8df8f88..7b3acb7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
*.exe
*.[aos]
*.py[co]
+*.o.d
*+
/config.mak
/autom4te.cache
diff --git a/Makefile b/Makefile
index 0e4e8ff..2e75f05 100644
--- a/Makefile
+++ b/Makefile
@@ -217,6 +217,10 @@ all::
# DEFAULT_EDITOR='~/bin/vi',
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
+#
+# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
+# and you want to avoid rebuilding objects when an unrelated header file
+# changes.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1677,14 +1681,48 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
ASM_OBJ := $(ASM_SRC:S=o)
C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+ifdef COMPUTE_HEADER_DEPENDENCIES
+dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS))))
+$(dep_dirs):
+ mkdir -p $@
+
+missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
+else
+dep_dirs =
+missing_dep_dirs =
+endif
+
.SUFFIXES:
-$(C_OBJ): %.o: %.c GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-$(ASM_OBJ): %.o: %.S GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+# Take advantage of gcc's on-the-fly dependency generation
+# See <http://gcc.gnu.org/gcc-3.0/features.html>.
+dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+else
+dep_args =
+
+# Dependencies on header files, for platforms that do not support
+# the gcc -MMD option.
+#
+# Dependencies on automatically generated headers such as common-cmds.h
+# should _not_ be included here, since they are necessary even when
+# building an object for the first time.
+#
+# XXX. Please check occasionally that these include all dependencies
+# gcc detects!
$(GIT_OBJS): $(LIB_H)
builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
@@ -1700,10 +1738,10 @@ builtin-pack-objects.o: thread-utils.h
http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
http.o http-walker.o http-push.o remote-curl.o: http.h
-
xdiff-interface.o $(XDIFF_OBJS): \
xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+endif
exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
@@ -2011,6 +2049,7 @@ clean:
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
$(RM) -r bin-wrappers
+ $(RM) -r $(dep_dirs)
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
$(RM) -r autom4te.cache
$(RM) config.log config.mak.autogen config.mak.append config.status config.cache
--
1.6.6
^ permalink raw reply related
* [PATCH 09/12] Makefile: list generated object files in OBJECTS
From: Jonathan Nieder @ 2010-01-26 15:52 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Set the OBJECTS variable to a comprehensive list of all object
file targets. To make sure it is truly comprehensive, restrict
the scope of the %.o pattern rule to only generate objects in
this list.
Attempts to build other object files will fail loudly:
$ touch foo.c
$ make foo.o
make: *** No rule to make target `foo.o'. Stop.
providing a reminder to add the new object to the OBJECTS list.
The new variable is otherwise unused. The intent is for later
patches to take advantage of it.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 60ef711..0e4e8ff 100644
--- a/Makefile
+++ b/Makefile
@@ -1671,14 +1671,19 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
$(patsubst git-%$X,%.o,$(PROGRAMS))
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
+
+ASM_SRC := $(wildcard $(OBJECTS:o=S))
+ASM_OBJ := $(ASM_SRC:S=o)
+C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
.SUFFIXES:
-%.o: %.c GIT-CFLAGS
+$(C_OBJ): %.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S GIT-CFLAGS
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
$(GIT_OBJS): $(LIB_H)
--
1.6.6
^ permalink raw reply related
* [PATCH 08/12] Makefile: disable default implicit rules
From: Jonathan Nieder @ 2010-01-26 15:51 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
The git makefile never uses any default implicit rules.
Unfortunately, if a prerequisite for one of the intended rules is
missing, a default rule can be used in its place:
$ make var.s
CC var.s
$ rm var.c
$ make var.o
as -o var.o var.s
Avoiding the default rules avoids this hard-to-debug behavior.
It also should speed things up a little in the normal case.
Future patches may restrict the scope of the %.o: %.c pattern.
This patch would then ensure that for targets not listed, we do
not fall back to the default rule.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Sadly, this does not really speed things up in the normal case.
I am not sure why, but make needs the -r option to avoid
considering the default rules, even though clearing the .SUFFIXES:
guarantees it will never use them.
Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 84ce137..60ef711 100644
--- a/Makefile
+++ b/Makefile
@@ -1672,6 +1672,8 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
+.SUFFIXES:
+
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
--
1.6.6
^ permalink raw reply related
* [PATCH 06/12] Makefile: transport.o depends on branch.h now
From: Jonathan Nieder @ 2010-01-26 15:47 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
Since commit e9fcd1e2 (Add push --set-upstream, 2010-01-16),
transport.c uses branch.h.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 59a46e2..5678991 100644
--- a/Makefile
+++ b/Makefile
@@ -1719,7 +1719,7 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
-builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
+builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
builtin-bundle.o bundle.o transport.o: bundle.h
builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
--
1.6.6
^ permalink raw reply related
* [PATCH 05/12] Makefile: drop dependency on $(wildcard */*.h)
From: Jonathan Nieder @ 2010-01-26 15:46 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
The files this pulls in are already pulled in by other dependency
rules (some recently added).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 593801a..98810b2 100644
--- a/Makefile
+++ b/Makefile
@@ -1655,7 +1655,7 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) $(wildcard */*.h)
+$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
builtin-bundle.o bundle.o transport.o: bundle.h
builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
--
1.6.6
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox