Re: [PATCH] Avoid a useless prefix lookup in strbuf_expand()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> writes:

> The loop makes implementing the callback function a bit easier, since
> you don't need to cover all cases; the input is already checked by
> strbuf_expand().
>
> Anyway, here's your patch again with a few small changes: the
> placeholders array is gone as you suggested, the cases for %Cx, %ax and
> %cx are check for unknown placeholders and the callback function returns
> the number of bytes it consumed as size_t.
>
> All in all: less code, slightly more complex callback functions (needs
> to return the length of the consumed placeholder or 0 if the input
> doesn't match a placeholder) and increased speed.  I have to admit that
> I start to like it. :-)

I'll let Marco bench it and hopefully Ack with an updated
(final) commit log message.

I think Dscho and Marco's earlier prefixcmp() optimization to
avoid strlen() can stay, but with "inline" removed.  That should
be equivalent to the version before the optimization, both from
the point of view of the code footprint and callchain length,
but still avoid strlen() cost.

Due to lack of better place the patch below moves it to strbuf.c
which probably is the closest collection of "stringy" stuff.

$ size git ;# with the attached patch
   text    data     bss     dec     hex filename
 731144   13456  263464 1008064   f61c0 git
$ size ../git.build/git ;# before Dscho's patch
   text    data     bss     dec     hex filename
 731272   13456  263464 1008192   f6240 ../git.build/git
$ size ~/bin/git ;# with Dscho's patch
   text    data     bss     dec     hex filename
 740736   13456  263464 1017656   f8738 /home/junio/bin/git

You earlier said 2,620,938 vs 2,640,450 and I think you meant
what "ls -l" reports.  I suspect it is not a very good measure,
but the numbers here are:

$ ls -l git ;# with the attached patch
-rwxrwxr-x 83 junio src 3345237 2008-01-03 00:53 git
$ ls -l ../git.build/git ;# before Dscho's patch
-rwxrwxr-x 83 junio src 3364803 2008-01-03 01:01 ../git.build/git
$ ls -l ~/bin/git ;# with Dscho's patch
-rwxr-xr-x 83 junio src 3389299 2008-01-02 15:18 /home/junio/bin/git

-- >8 --
Uninline prefixcmp()

Now the routine is an open-coded loop that avoids an extra
strlen() in the previous implementation, it got a bit too big to
be inlined.  Uninlining it makes code footprint smaller but the
result still retains the avoidance of strlen() cost.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 git-compat-util.h |   11 ++---------
 strbuf.c          |    9 +++++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 7059cbd..b6ef544 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -122,6 +122,8 @@ extern void set_die_routine(void (*routine)(const char *err, va_list params) NOR
 extern void set_error_routine(void (*routine)(const char *err, va_list params));
 extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
 
+extern int prefixcmp(const char *str, const char *prefix);
+
 #ifdef NO_MMAP
 
 #ifndef PROT_READ
@@ -396,15 +398,6 @@ static inline int sane_case(int x, int high)
 	return x;
 }
 
-static inline int prefixcmp(const char *str, const char *prefix)
-{
-	for (; ; str++, prefix++)
-		if (!*prefix)
-			return 0;
-		else if (*str != *prefix)
-			return (unsigned char)*prefix - (unsigned char)*str;
-}
-
 static inline int strtoul_ui(char const *s, int base, unsigned int *result)
 {
 	unsigned long ul;
diff --git a/strbuf.c b/strbuf.c
index b9b194b..5efcfc8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,5 +1,14 @@
 #include "cache.h"
 
+int prefixcmp(const char *str, const char *prefix)
+{
+	for (; ; str++, prefix++)
+		if (!*prefix)
+			return 0;
+		else if (*str != *prefix)
+			return (unsigned char)*prefix - (unsigned char)*str;
+}
+
 /*
  * Used as the default ->buf value, so that people can always assume
  * buf is non NULL and ->buf is NUL terminated even for a freshly
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux