[Rpm-maint] [PATCH 3/5] Optimize expandMacro function

[Alexey Tourbin]

From: Alexey Tourbin <at at altlinux.ru>

This change brings the following optimizations:
- In the beginning of the routine, the input string needs to be copied
  only in non-terminated case.
- When mb->buf is NULL, expensive xcalloc call is replaced with xmalloc.
  It suffices to ensure that mb->buf is (initially) null-terminated at
  mb->tpos, and that mbAppend* routines follow this rule.
- The inner per-character loop is replaced with call to strchr(s, '%').
  Also note that the "trailing %" case is no longer handled by this part
  of the code (this is now the case of no valid macro name).
- New routine mbAppendMem now complements mbAppendStr.  Also, mbGrow
  routine was factored to improve possibilities for inlining.

Callgrind annotations for 'rpmspec -P selinux-policy.spec', previous commit:
160,979,831  PROGRAM TOTALS
58,348,925  rpmio/argv.c:argvCount
14,378,832  build/parseSpec.c:readLine
10,710,181  glibc-2.16-75f0d304/string/../sysdeps/x86_64/memset.S:__GI_memset
 5,445,134  rpmio/macro.c:mbAppend
 5,147,651  glibc-2.16-75f0d304/malloc/malloc.c:_int_malloc
 3,490,055  rpm-4.10.2/lib/header.c:copyData
...

Callgrind annotations for 'rpmspec -P selinux-policy.spec', this commit:
140,610,489  PROGRAM TOTALS
58,348,925  rpmio/argv.c:argvCount
14,378,832  build/parseSpec.c:readLine
 3,893,942  glibc-2.16-75f0d304/malloc/malloc.c:_int_malloc
 3,490,055  rpm-4.10.2/lib/header.c:copyData
...
---
 rpmio/macro.c | 94 ++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 33 deletions(-)

diff --git a/rpmio/macro.c b/rpmio/macro.c
index 0b1aacb..eee7716 100644
--- a/rpmio/macro.c
+++ b/rpmio/macro.c
@@ -369,12 +369,16 @@ expandThis(MacroBuf mb, const char * src, size_t slen, char **target)
     return rc;
 }
 
+static void mbGrow(MacroBuf mb, size_t n)
+{
+    mb->buf = xrealloc(mb->buf, mb->tpos + MACROBUFSIZ + n + 1);
+    mb->nb += MACROBUFSIZ + n;
+}
+
 static void mbAppend(MacroBuf mb, char c)
 {
-    if (mb->nb < 1) {
-	mb->buf = xrealloc(mb->buf, mb->tpos + MACROBUFSIZ + 1);
-	mb->nb += MACROBUFSIZ;
-    }
+    if (mb->nb < 1)
+	mbGrow(mb, 1);
     mb->buf[mb->tpos++] = c;
     mb->buf[mb->tpos] = '\0';
     mb->nb--;
@@ -383,14 +387,23 @@ static void mbAppend(MacroBuf mb, char c)
 static void mbAppendStr(MacroBuf mb, const char *str)
 {
     size_t len = strlen(str);
-    if (len > mb->nb) {
-	mb->buf = xrealloc(mb->buf, mb->tpos + mb->nb + MACROBUFSIZ + len + 1);
-	mb->nb += MACROBUFSIZ + len;
-    }
-    memcpy(mb->buf+mb->tpos, str, len + 1);
+    if (mb->nb < len)
+	mbGrow(mb, len);
+    memcpy(mb->buf + mb->tpos, str, len + 1);
     mb->tpos += len;
     mb->nb -= len;
 }
+
+static void mbAppendMem(MacroBuf mb, const char *str, size_t len)
+{
+    if (mb->nb < len)
+	mbGrow(mb, len);
+    memcpy(mb->buf + mb->tpos, str, len);
+    mb->tpos += len;
+    mb->buf[mb->tpos] = '\0';
+    mb->nb -= len;
+}
+
 /**
  * Expand output of shell command into target buffer.
  * @param mb		macro expansion state
@@ -990,18 +1003,19 @@ expandMacro(MacroBuf mb, const char *src, size_t slen)
     char *source = NULL;
 
     /* Handle non-terminated substrings by creating a terminated copy */
-    if (!slen)
-	slen = strlen(src);
-    source = xmalloc(slen + 1);
-    strncpy(source, src, slen);
-    source[slen] = '\0';
-    s = source;
+    if (slen) {
+	source = xmalloc(slen + 1);
+	memcpy(source, src, slen);
+	source[slen] = '\0';
+	s = source;
+    }
 
     if (mb->buf == NULL) {
-	size_t blen = MACROBUFSIZ + strlen(s);
-	mb->buf = xcalloc(blen + 1, sizeof(*mb->buf));
-	mb->tpos = 0;
+	size_t blen = MACROBUFSIZ + slen ? slen : strlen(s);
+	mb->buf = xmalloc(blen + 1);
+	mb->buf[0] = '\0';
 	mb->nb = blen;
+	mb->tpos = 0;
     }
     tpos = mb->tpos; /* save expansion pointer for printExpand */
 
@@ -1014,23 +1028,38 @@ expandMacro(MacroBuf mb, const char *src, size_t slen)
 	return 1;
     }
 
-    while (rc == 0 && (c = *s) != '\0') {
-	s++;
-	/* Copy text until next macro */
-	switch(c) {
-	case '%':
-		if (*s) {	/* Ensure not end-of-string. */
-		    if (*s != '%')
-			break;
-		    s++;	/* skip first % in %% */
-		}
-	default:
-		mbAppend(mb, c);
-		continue;
+    while (rc == 0 && *s != '\0') {
+	/* Scan for % */
+	se = strchr(s, '%');
+	/* End of input */
+	if (se == NULL) {
+	    mbAppendStr(mb, s);
+	    break;
+	}
+	/* Copy text and expand %% to % */
+	if (se[1] == '%') {
+	    size_t len = se - s + 1;
+	    mbAppendMem(mb, s, len);
+	    s += len + 1;
+	    continue;
+	}
+	/* Copy text and handle the macro */
+	else {
+	    size_t len = se - s;
+	    switch (len) {
+	    case 0: /* optimize for e.g. %{buildroot}%{_libdir} */
+		break;
+	    case 1: /* optimize for e.g. %{name}-%{version} */
+		mbAppend(mb, *s);
 		break;
+	    default:
+		mbAppendMem(mb, s, len);
+		break;
+	    }
+	    s += len + 1;
 	}
 
-	/* Expand next macro */
+	/* Expand the macro */
 	f = fe = NULL;
 	g = ge = NULL;
 	if (mb->depth > 1)	/* XXX full expansion for outermost level */
@@ -1282,7 +1311,6 @@ expandMacro(MacroBuf mb, const char *src, size_t slen)
 	s = se;
     }
 
-    mb->buf[mb->tpos] = '\0';
     mb->depth--;
     if (rc != 0 || mb->expand_trace)
 	printExpansion(mb, mb->buf+tpos, mb->buf+mb->tpos);
-- 
1.8.1