From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Thu, 15 Jul 2021 03:23:31 +0000 (-0700)
Subject: str_ior: [nfc] pre-compute and allocate the result string
X-Git-Tag: 2.49.0~52^2~8
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a0f7949ab68fd057e6a722be6d849110fa840b83;p=graphviz

str_ior: [nfc] pre-compute and allocate the result string

This change does not affect the functionality of this function, but it has two
motivating advantages:

  1. The temporary scratch buffer `ex->tmp` is no longer used. Though it is not
     obvious without auditing a lot of surrounding code, the data written into
     this buffer does not need to be retained beyond the lifetime of this
     function. Removing its use not only removes a code path through sfio, but
     decouples this code from other code using `ex->tmp` making it easier to
     understand. Related to #1873, #1998.

  2. The prior code used an sfio temporary buffer to construct the result string
     and then duplicated it into a vmalloc-allocated buffer. This is reasonable
     as vmalloc has no support for incrementally constructing dynamically
     allocated strings. However we can avoid the intermediate sfio buffer by
     simply pre-computing the final vmalloc allocation that will be needed. This
     change does exactly that and simply writes the result once into its final
     destination instead of copying through an intermediate buffer. This
     should not only (slightly) decrease transient heap pressure, but also
     (again slightly) accelerate the performance of this function.

Both these effects are a simplification with respect to how the compiler sees
this function. That is, an optimizing compiler should now better comprehend the
intent of this function and be able to more aggressively specialize and inline
it where relevant.
---

diff --git a/lib/expr/exeval.c b/lib/expr/exeval.c
index 4ffb66074..090c4b5b1 100644
--- a/lib/expr/exeval.c
+++ b/lib/expr/exeval.c
@@ -564,17 +564,45 @@ str_add(Expr_t* ex, char* l, char* r)
 
 static char *str_ior(Expr_t *ex, const char *l, const char *r) {
 
+  // compute how much space the result will occupy
+  size_t len = 1; // 1 for NUL terminator
   for (const char *p = l; *p != '\0'; ++p) {
     if (strchr(p + 1, *p) == NULL) {
-      sfputc(ex->tmp, *p);
+      ++len;
     }
   }
   for (const char *p = r; *p != '\0'; ++p) {
     if (strchr(l, *p) == NULL && strchr(p + 1, *p) == NULL) {
-      sfputc(ex->tmp, *p);
+      ++len;
     }
   }
-  return exstash(ex->tmp, ex->ve);
+
+  // allocate a buffer to store this
+  char *result = vmalloc(ex->ve, len);
+  if (result == NULL) {
+    return exnospace();
+  }
+
+  // write the result
+  size_t i = 0;
+  for (const char *p = l; *p != '\0'; ++p) {
+    if (strchr(p + 1, *p) == NULL) {
+      assert(i < len && "incorrect preceding length computation");
+      result[i] = *p;
+      ++i;
+    }
+  }
+  for (const char *p = r; *p != '\0'; ++p) {
+    if (strchr(l, *p) == NULL && strchr(p + 1, *p) == NULL) {
+      assert(i < len && "incorrect preceding length computation");
+      result[i] = *p;
+      ++i;
+    }
+  }
+  assert(i + 1 == len && "incorrect preceding length computation");
+  result[i] = '\0';
+
+  return result;
 }
 
 /*