From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sun, 7 Aug 2022 16:24:43 +0000 (-0700)
Subject: gvpr toLower: rewrite to use a single buffer
X-Git-Tag: 5.0.1~9^2~8
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dea0343e21032f0f29ec42cbc0c099ff00391d6f;p=graphviz

gvpr toLower: rewrite to use a single buffer

`toLower` worked by using the GVPR state’s temporary buffer, `tmp`, to
incrementally construct a lowercase copy of the input. This works but is
suboptimal. When printing to the temporary buffer, it has no knowledge of what
we are up to and needs to make heuristic-based decisions on how to allocate
memory. But we know the exact size of the result string already. This commit
takes advantage of this knowledge and allocates the destination memory upfront
and then simply writes directly into it. By doing this, we reduce heap pressure
and eliminate an intermediate copying operation.

This commit also renames the `s` parameter to something more descriptive.

Gitlab: #1873
---

diff --git a/lib/gvpr/actions.c b/lib/gvpr/actions.c
index e380e0668..c2c8688bc 100644
--- a/lib/gvpr/actions.c
+++ b/lib/gvpr/actions.c
@@ -24,6 +24,7 @@
 #include <stdio.h>
 #include <ctype.h>
 #include <cgraph/agxbuf.h>
+#include <cgraph/likely.h>
 #include <cgraph/strcasecmp.h>
 #include <cgraph/unreachable.h>
 
@@ -762,14 +763,20 @@ int compare(Agobj_t * l, Agobj_t * r)
 /* toLower:
  * Convert characters to lowercase
  */
-char *toLower(Expr_t * pgm, char *s, Sfio_t* tmps)
-{
-    int c;
+char *toLower(Expr_t *pgm, char *src) {
 
-    while ((c = *s++))
-	sfputc (tmps, tolower (c));
+  const size_t len = strlen(src);
+  char *dst = exstralloc(pgm, len + 1);
+  if (UNLIKELY(dst == NULL)) {
+    return NULL;
+  }
 
-    return exstring(pgm, sfstruse(tmps));
+  for (size_t i = 0; i < len; ++i) {
+    dst[i] = (char)tolower((int)src[i]);
+  }
+
+  dst[len] = '\0';
+  return dst;
 }
 
 /* toUpper:
diff --git a/lib/gvpr/actions.h b/lib/gvpr/actions.h
index 39dc11787..98941e632 100644
--- a/lib/gvpr/actions.h
+++ b/lib/gvpr/actions.h
@@ -42,7 +42,7 @@ extern "C" {
     extern char *readLine(Expr_t *, int);
     extern char *canon(Expr_t * pgm, char *);
     extern char *toHtml(Agraph_t*, char *);
-    extern char *toLower(Expr_t * pgm, char *, Sfio_t*);
+    extern char *toLower(Expr_t *pgm, char *src);
     extern char *toUpper(Expr_t * pgm, char *, Sfio_t*);
     extern int deleteObj(Agraph_t * g, Agobj_t * obj);
     extern char *colorx (Expr_t* ex, char* incolor, char* fmt, Sfio_t* fp);
diff --git a/lib/gvpr/compile.c b/lib/gvpr/compile.c
index 3c4527970..62fc8dcfe 100644
--- a/lib/gvpr/compile.c
+++ b/lib/gvpr/compile.c
@@ -1446,7 +1446,7 @@ getval(Expr_t * pgm, Exnode_t * node, Exid_t * sym, Exref_t * ref,
 	    }
 	    break;
 	case F_tolower:
-	    v.string = toLower(pgm, args[0].string, state->tmp);
+	    v.string = toLower(pgm, args[0].string);
 	    break;
 	case F_colorx:
 	    v.string = colorx(pgm, args[0].string, args[1].string, state->tmp);