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

René Scharfe [off-list ref] 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 <redacted>
---
 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

On Jan 3, 2008 10:08 AM, Junio C Hamano [off-list ref] wrote:

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

I will bench today and post the results.

Marco