refactor(api): reduce temporary allocations when replacing lines

The way ml_replace_buf is implemented makes it unfriendly for
being used in a loop: every call allocates a scratch buffer for putting
the line into the "dirty" state. This then immediately needs to be freed
as the next ml_replace_buf and/or ml_append_buf call will flush that buffer.

It's better to later pay the price of allocating the scratch buffer only if
the line is being immediately edited (likely when using the API to only
change one line) with an extra memcpy, than allocating that buffer
multiple times every time the API is called.

Of course, a separate xmalloc/xfree cycle for each time the dirty line
changes is unwanted to begin with. But fixing that is a later refactor.

1 files changed, 21 insertions, 46 deletions

diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c
index ddca85945a..ad25451054 100644
--- a/src/nvim/api/buffer.c
+++ b/src/nvim/api/buffer.c
@@ -325,7 +325,8 @@ ArrayOf(String) nvim_buf_get_lines(uint64_t channel_id,
 /// @param replacement      Array of lines to use as replacement
 /// @param[out] err         Error details, if any
 void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integer end,
-                        Boolean strict_indexing, ArrayOf(String) replacement, Error *err)
+                        Boolean strict_indexing, ArrayOf(String) replacement, Arena *arena,
+                        Error *err)
   FUNC_API_SINCE(1)
   FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
@@ -360,14 +361,14 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
   size_t new_len = replacement.size;
   size_t old_len = (size_t)(end - start);
   ptrdiff_t extra = 0;  // lines added to text, can be negative
-  char **lines = (new_len != 0) ? xcalloc(new_len, sizeof(char *)) : NULL;
+  char **lines = (new_len != 0) ? arena_alloc(arena, new_len * sizeof(char *), true) : NULL;
 
   for (size_t i = 0; i < new_len; i++) {
     const String l = replacement.items[i].data.string;
 
     // Fill lines[i] with l's contents. Convert NULs to newlines as required by
     // NL-used-for-NUL.
-    lines[i] = xmemdupz(l.data, l.size);
+    lines[i] = arena_memdupz(arena, l.data, l.size);
     memchrsub(lines[i], NUL, NL, l.size);
   }
 
@@ -412,15 +413,12 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
       goto end;
     });
 
-    if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false) == FAIL) {
+    if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false, true) == FAIL) {
       api_set_error(err, kErrorTypeException, "Failed to replace line");
       goto end;
     }
 
     inserted_bytes += (bcount_t)strlen(lines[i]) + 1;
-    // Mark lines that haven't been passed to the buffer as they need
-    // to be freed later
-    lines[i] = NULL;
   }
 
   // Now we may need to insert the remaining new old_len
@@ -438,9 +436,6 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
 
     inserted_bytes += (bcount_t)strlen(lines[i]) + 1;
 
-    // Same as with replacing, but we also need to free lines
-    xfree(lines[i]);
-    lines[i] = NULL;
     extra++;
   }
 
@@ -463,11 +458,6 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
   }
 
 end:
-  for (size_t i = 0; i < new_len; i++) {
-    xfree(lines[i]);
-  }
-
-  xfree(lines);
   try_end(err);
 }
 
@@ -500,7 +490,8 @@ end:
 /// @param replacement      Array of lines to use as replacement
 /// @param[out] err         Error details, if any
 void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, Integer start_col,
-                       Integer end_row, Integer end_col, ArrayOf(String) replacement, Error *err)
+                       Integer end_row, Integer end_col, ArrayOf(String) replacement, Arena *arena,
+                       Error *err)
   FUNC_API_SINCE(7)
   FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
@@ -535,33 +526,31 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
     return;
   });
 
-  char *str_at_start = NULL;
-  char *str_at_end = NULL;
-
-  // Another call to ml_get_buf() may free the line, so make a copy.
-  str_at_start = xstrdup(ml_get_buf(buf, (linenr_T)start_row));
+  // Another call to ml_get_buf() may free the lines, so we make copies
+  char *str_at_start = ml_get_buf(buf, (linenr_T)start_row);
   size_t len_at_start = strlen(str_at_start);
+  str_at_start = arena_memdupz(arena, str_at_start, len_at_start);
   start_col = start_col < 0 ? (int64_t)len_at_start + start_col + 1 : start_col;
   VALIDATE_RANGE((start_col >= 0 && (size_t)start_col <= len_at_start), "start_col", {
-    goto early_end;
+    return;
   });
 
-  // Another call to ml_get_buf() may free the line, so make a copy.
-  str_at_end = xstrdup(ml_get_buf(buf, (linenr_T)end_row));
+  char *str_at_end = ml_get_buf(buf, (linenr_T)end_row);
   size_t len_at_end = strlen(str_at_end);
+  str_at_end = arena_memdupz(arena, str_at_end, len_at_end);
   end_col = end_col < 0 ? (int64_t)len_at_end + end_col + 1 : end_col;
   VALIDATE_RANGE((end_col >= 0 && (size_t)end_col <= len_at_end), "end_col", {
-    goto early_end;
+    return;
   });
 
   VALIDATE((start_row <= end_row && !(end_row == start_row && start_col > end_col)),
            "%s", "'start' is higher than 'end'", {
-    goto early_end;
+    return;
   });
 
   bool disallow_nl = (channel_id != VIML_INTERNAL_CALL);
   if (!check_string_array(replacement, "replacement string", disallow_nl, err)) {
-    goto early_end;
+    return;
   }
 
   size_t new_len = replacement.size;
@@ -591,7 +580,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
   if (replacement.size == 1) {
     firstlen += last_part_len;
   }
-  char *first = xmallocz(firstlen);
+  char *first = arena_allocz(arena, firstlen);
   char *last = NULL;
   memcpy(first, str_at_start, (size_t)start_col);
   memcpy(first + start_col, first_item.data, first_item.size);
@@ -599,13 +588,13 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
   if (replacement.size == 1) {
     memcpy(first + start_col + first_item.size, str_at_end + end_col, last_part_len);
   } else {
-    last = xmallocz(last_item.size + last_part_len);
+    last = arena_allocz(arena, last_item.size + last_part_len);
     memcpy(last, last_item.data, last_item.size);
     memchrsub(last, NUL, NL, last_item.size);
     memcpy(last + last_item.size, str_at_end + end_col, last_part_len);
   }
 
-  char **lines = xcalloc(new_len, sizeof(char *));
+  char **lines = arena_alloc(arena, new_len * sizeof(char *), true);
   lines[0] = first;
   new_byte += (bcount_t)(first_item.size);
   for (size_t i = 1; i < new_len - 1; i++) {
@@ -613,7 +602,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
 
     // Fill lines[i] with l's contents. Convert NULs to newlines as required by
     // NL-used-for-NUL.
-    lines[i] = xmemdupz(l.data, l.size);
+    lines[i] = arena_memdupz(arena, l.data, l.size);
     memchrsub(lines[i], NUL, NL, l.size);
     new_byte += (bcount_t)(l.size) + 1;
   }
@@ -665,13 +654,10 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
       goto end;
     });
 
-    if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false) == FAIL) {
+    if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false, true) == FAIL) {
       api_set_error(err, kErrorTypeException, "Failed to replace line");
       goto end;
     }
-    // Mark lines that haven't been passed to the buffer as they need
-    // to be freed later
-    lines[i] = NULL;
   }
 
   // Now we may need to insert the remaining new old_len
@@ -687,9 +673,6 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
       goto end;
     }
 
-    // Same as with replacing, but we also need to free lines
-    xfree(lines[i]);
-    lines[i] = NULL;
     extra++;
   }
 
@@ -722,15 +705,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
   }
 
 end:
-  for (size_t i = 0; i < new_len; i++) {
-    xfree(lines[i]);
-  }
-  xfree(lines);
   try_end(err);
-
-early_end:
-  xfree(str_at_start);
-  xfree(str_at_end);
 }
 
 /// Gets a range from the buffer.