From 8bad6d065cc3cea439350983de7d5594bf32ed34 Mon Sep 17 00:00:00 2001
From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Wed, 14 Jul 2021 21:32:52 -0700
Subject: [PATCH] str_mpy: [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.
---
 lib/expr/exeval.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/lib/expr/exeval.c b/lib/expr/exeval.c
index 64ffc469a..f91e3fa1b 100644
--- a/lib/expr/exeval.c
+++ b/lib/expr/exeval.c
@@ -728,10 +728,32 @@ static char *str_mod(Expr_t *ex, const char *l, const char *r) {
 
 static char *str_mpy(Expr_t *ex, const char *l, const char *r) {
 
-  for (size_t i = 0; l[i] != '\0' && r[i] != '\0'; ++i) {
-    sfputc(ex->tmp, l[i] == r[i] ? l[i] : ' ');
+  // compute how much space the result will occupy
+  size_t len = strlen(l);
+  {
+    size_t len2 = strlen(r);
+    if (len2 < len) {
+      len = len2;
+    }
+  }
+  ++len; // 1 for NUL terminator
+
+  // allocate a buffer to store this
+  char *result = vmalloc(ex->ve, len);
+  if (result == NULL) {
+    return exnospace();
   }
-  return exstash(ex->tmp, ex->ve);
+
+  // write the result
+  size_t i = 0;
+  for (; l[i] != '\0' && r[i] != '\0'; ++i) {
+    assert(i < len && "incorrect preceding length computation");
+    result[i] = l[i] == r[i] ? l[i] : ' ';
+  }
+  assert(i + 1 == len && "incorrect preceding length computation");
+  result[i] = '\0';
+
+  return result;
 }
 
 /* replace:
-- 
2.40.0