src: use string_view for report and related code

Use `std::string_view` for process.report code and related code, drop a few unnecessary `std::to_string` calls, and use `MaybeStackBuffer` instead of `MallocedBuffer`, all in order to avoid unnecessary heap allocations. PR-URL: nodejs#46723 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
anonrig · Apr 6, 2023 · 910d27e · 910d27e
1 parent 574f464
commit 910d27e
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 64 deletions.
diff --git a/src/json_utils.cc b/src/json_utils.cc
@@ -2,8 +2,8 @@
 
 namespace node {
 
-std::string EscapeJsonChars(const std::string& str) {
-  const std::string control_symbols[0x20] = {
+std::string EscapeJsonChars(std::string_view str) {
+  static const std::string_view control_symbols[0x20] = {
       "\\u0000", "\\u0001", "\\u0002", "\\u0003", "\\u0004", "\\u0005",
       "\\u0006", "\\u0007", "\\b",     "\\t",     "\\n",     "\\u000b",
       "\\f",     "\\r",     "\\u000e", "\\u000f", "\\u0010", "\\u0011",

diff --git a/src/json_utils.h b/src/json_utils.h
@@ -4,13 +4,21 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include <iomanip>
-#include <ostream>
 #include <limits>
+#include <ostream>
 #include <string>
+#include <string_view>
 
 namespace node {
 
-std::string EscapeJsonChars(const std::string& str);
+constexpr bool NeedsJsonEscape(std::string_view str) {
+  for (const char c : str) {
+    if (c == '\\' || c == '"' || c < 0x20) return true;
+  }
+  return false;
+}
+
+std::string EscapeJsonChars(std::string_view str);
 std::string Reindent(const std::string& str, int indentation);
 
 // JSON compiler definitions.
@@ -135,17 +143,20 @@ class JSONWriter {
   }
 
   inline void write_value(Null null) { out_ << "null"; }
-  inline void write_value(const char* str) { write_string(str); }
-  inline void write_value(const std::string& str) { write_string(str); }
+  inline void write_value(std::string_view str) { write_string(str); }
 
   inline void write_value(const ForeignJSON& json) {
     out_ << Reindent(json.as_string, indent_);
   }
 
-  inline void write_string(const std::string& str) {
-    out_ << '"' << EscapeJsonChars(str) << '"';
+  inline void write_string(std::string_view str) {
+    out_ << '"';
+    if (NeedsJsonEscape(str))  // only create temporary std::string if necessary
+      out_ << EscapeJsonChars(str);
+    else
+      out_ << str;
+    out_ << '"';
   }
-  inline void write_string(const char* str) { write_string(std::string(str)); }
 
   enum JSONState { kObjectStart, kAfterValue };
   std::ostream& out_;

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -657,7 +657,7 @@ void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
     // a file system path.
     // TODO(gabrielschulhof): Pass the `filename` through unchanged if/when we
     // receive it as a URL already.
-    module_filename = node::url::FromFilePath(filename.ToString());
+    module_filename = node::url::FromFilePath(filename.ToStringView());
   }
 
   // Create a new napi_env for this specific module.

diff --git a/src/node_errors.cc b/src/node_errors.cc
@@ -420,8 +420,10 @@ static void ReportFatalException(Environment* env,
       // Not an error object. Just print as-is.
       node::Utf8Value message(env->isolate(), error);
 
-      FPrintF(stderr, "%s\n",
-              *message ? message.ToString() : "<toString() threw exception>");
+      FPrintF(
+          stderr,
+          "%s\n",
+          *message ? message.ToStringView() : "<toString() threw exception>");
     } else {
       node::Utf8Value name_string(env->isolate(), name.ToLocalChecked());
       node::Utf8Value message_string(env->isolate(), message.ToLocalChecked());

diff --git a/src/node_report.cc b/src/node_report.cc
@@ -400,11 +400,10 @@ static void PrintJavaScriptErrorProperties(JSONWriter* writer,
           !value->ToString(context).ToLocal(&value_string)) {
         continue;
       }
-      String::Utf8Value k(isolate, key);
+      node::Utf8Value k(isolate, key);
       if (!strcmp(*k, "stack") || !strcmp(*k, "message")) continue;
-      String::Utf8Value v(isolate, value_string);
-      writer->json_keyvalue(std::string(*k, k.length()),
-                            std::string(*v, v.length()));
+      node::Utf8Value v(isolate, value_string);
+      writer->json_keyvalue(k.ToStringView(), v.ToStringView());
     }
   }
   writer->json_objectend();  // the end of 'errorProperties'
@@ -631,27 +630,26 @@ static void PrintResourceUsage(JSONWriter* writer) {
   uint64_t free_memory = uv_get_free_memory();
   uint64_t total_memory = uv_get_total_memory();
 
-  writer->json_keyvalue("free_memory", std::to_string(free_memory));
-  writer->json_keyvalue("total_memory", std::to_string(total_memory));
+  writer->json_keyvalue("free_memory", free_memory);
+  writer->json_keyvalue("total_memory", total_memory);
 
   size_t rss;
   int err = uv_resident_set_memory(&rss);
   if (!err) {
-    writer->json_keyvalue("rss", std::to_string(rss));
+    writer->json_keyvalue("rss", rss);
   }
 
   uint64_t constrained_memory = uv_get_constrained_memory();
   if (constrained_memory) {
-    writer->json_keyvalue("constrained_memory",
-                          std::to_string(constrained_memory));
+    writer->json_keyvalue("constrained_memory", constrained_memory);
   }
 
   // See GuessMemoryAvailableToTheProcess
   if (!err && constrained_memory && constrained_memory >= rss) {
     uint64_t available_memory = constrained_memory - rss;
-    writer->json_keyvalue("available_memory", std::to_string(available_memory));
+    writer->json_keyvalue("available_memory", available_memory);
   } else {
-    writer->json_keyvalue("available_memory", std::to_string(free_memory));
+    writer->json_keyvalue("available_memory", free_memory);
   }
 
   if (uv_getrusage(&rusage) == 0) {
@@ -668,7 +666,7 @@ static void PrintResourceUsage(JSONWriter* writer) {
     writer->json_keyvalue("cpuConsumptionPercent", cpu_percentage);
     writer->json_keyvalue("userCpuConsumptionPercent", user_cpu_percentage);
     writer->json_keyvalue("kernelCpuConsumptionPercent", kernel_cpu_percentage);
-    writer->json_keyvalue("maxRss", std::to_string(rusage.ru_maxrss * 1024));
+    writer->json_keyvalue("maxRss", rusage.ru_maxrss * 1024);
     writer->json_objectstart("pageFaults");
     writer->json_keyvalue("IORequired", rusage.ru_majflt);
     writer->json_keyvalue("IONotRequired", rusage.ru_minflt);
@@ -795,13 +793,15 @@ static void PrintComponentVersions(JSONWriter* writer) {
   writer->json_objectstart("componentVersions");
 
 #define V(key) +1
-  std::pair<std::string, std::string> versions_array[NODE_VERSIONS_KEYS(V)];
+  std::pair<std::string_view, std::string_view>
+      versions_array[NODE_VERSIONS_KEYS(V)];
 #undef V
   auto* slot = &versions_array[0];
 
 #define V(key)                                                                 \
   do {                                                                         \
-    *slot++ = std::make_pair(#key, per_process::metadata.versions.key);        \
+    *slot++ = std::pair<std::string_view, std::string_view>(                   \
+        #key, per_process::metadata.versions.key);                             \
   } while (0);
   NODE_VERSIONS_KEYS(V)
 #undef V

diff --git a/src/node_report_utils.cc b/src/node_report_utils.cc
@@ -83,79 +83,75 @@ static void ReportEndpoints(uv_handle_t* h, JSONWriter* writer) {
 // Utility function to format libuv pipe information.
 static void ReportPipeEndpoints(uv_handle_t* h, JSONWriter* writer) {
   uv_any_handle* handle = reinterpret_cast<uv_any_handle*>(h);
-  MallocedBuffer<char> buffer(0);
-  size_t buffer_size = 0;
+  MaybeStackBuffer<char> buffer;
+  size_t buffer_size = buffer.capacity();
   int rc = -1;
 
   // First call to get required buffer size.
-  rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size);
+  rc = uv_pipe_getsockname(&handle->pipe, buffer.out(), &buffer_size);
   if (rc == UV_ENOBUFS) {
-    buffer = MallocedBuffer<char>(buffer_size);
-    if (buffer.data != nullptr) {
-      rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size);
-    } else {
-      buffer_size = 0;
-    }
+    buffer.AllocateSufficientStorage(buffer_size);
+    rc = uv_pipe_getsockname(&handle->pipe, buffer.out(), &buffer_size);
   }
-  if (rc == 0 && buffer_size != 0 && buffer.data != nullptr) {
-    writer->json_keyvalue("localEndpoint", buffer.data);
+  if (rc == 0 && buffer_size != 0) {
+    buffer.SetLength(buffer_size);
+    writer->json_keyvalue("localEndpoint", buffer.ToStringView());
   } else {
     writer->json_keyvalue("localEndpoint", null);
   }
 
   // First call to get required buffer size.
-  rc = uv_pipe_getpeername(&handle->pipe, buffer.data, &buffer_size);
+  buffer_size = buffer.capacity();
+  rc = uv_pipe_getpeername(&handle->pipe, buffer.out(), &buffer_size);
   if (rc == UV_ENOBUFS) {
-    buffer = MallocedBuffer<char>(buffer_size);
-    if (buffer.data != nullptr) {
-      rc = uv_pipe_getpeername(&handle->pipe, buffer.data, &buffer_size);
-    }
+    buffer.AllocateSufficientStorage(buffer_size);
+    rc = uv_pipe_getpeername(&handle->pipe, buffer.out(), &buffer_size);
   }
-  if (rc == 0 && buffer_size != 0 && buffer.data != nullptr) {
-    writer->json_keyvalue("remoteEndpoint", buffer.data);
+  if (rc == 0 && buffer_size != 0) {
+    buffer.SetLength(buffer_size);
+    writer->json_keyvalue("remoteEndpoint", buffer.ToStringView());
   } else {
     writer->json_keyvalue("remoteEndpoint", null);
   }
 }
 
 // Utility function to format libuv path information.
 static void ReportPath(uv_handle_t* h, JSONWriter* writer) {
-  MallocedBuffer<char> buffer(0);
+  MaybeStackBuffer<char> buffer;
   int rc = -1;
-  size_t size = 0;
+  size_t size = buffer.capacity();
   uv_any_handle* handle = reinterpret_cast<uv_any_handle*>(h);
-  bool wrote_filename = false;
   // First call to get required buffer size.
   switch (h->type) {
     case UV_FS_EVENT:
-      rc = uv_fs_event_getpath(&(handle->fs_event), buffer.data, &size);
+      rc = uv_fs_event_getpath(&(handle->fs_event), buffer.out(), &size);
       break;
     case UV_FS_POLL:
-      rc = uv_fs_poll_getpath(&(handle->fs_poll), buffer.data, &size);
+      rc = uv_fs_poll_getpath(&(handle->fs_poll), buffer.out(), &size);
       break;
     default:
       break;
   }
   if (rc == UV_ENOBUFS) {
-    buffer = MallocedBuffer<char>(size + 1);
+    buffer.AllocateSufficientStorage(size);
     switch (h->type) {
       case UV_FS_EVENT:
-        rc = uv_fs_event_getpath(&(handle->fs_event), buffer.data, &size);
+        rc = uv_fs_event_getpath(&(handle->fs_event), buffer.out(), &size);
         break;
       case UV_FS_POLL:
-        rc = uv_fs_poll_getpath(&(handle->fs_poll), buffer.data, &size);
+        rc = uv_fs_poll_getpath(&(handle->fs_poll), buffer.out(), &size);
         break;
       default:
         break;
     }
-    if (rc == 0) {
-      // buffer is not null terminated.
-      buffer.data[size] = '\0';
-      writer->json_keyvalue("filename", buffer.data);
-      wrote_filename = true;
-    }
   }
-  if (!wrote_filename) writer->json_keyvalue("filename", null);
+
+  if (rc == 0 && size > 0) {
+    buffer.SetLength(size);
+    writer->json_keyvalue("filename", buffer.ToStringView());
+  } else {
+    writer->json_keyvalue("filename", null);
+  }
 }
 
 // Utility function to walk libuv handles.

diff --git a/src/util.h b/src/util.h
@@ -37,10 +37,10 @@
 #include <array>
 #include <limits>
 #include <memory>
+#include <set>
 #include <string>
 #include <string_view>
 #include <type_traits>
-#include <set>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -485,6 +485,11 @@ class MaybeStackBuffer {
       free(buf_);
   }
 
+  inline std::basic_string<T> ToString() const { return {out(), length()}; }
+  inline std::basic_string_view<T> ToStringView() const {
+    return {out(), length()};
+  }
+
  private:
   size_t length_;
   // capacity of the malloc'ed buf_
@@ -532,8 +537,6 @@ class Utf8Value : public MaybeStackBuffer<char> {
  public:
   explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
 
-  inline std::string ToString() const { return std::string(out(), length()); }
-
   inline bool operator==(const char* a) const {
     return strcmp(out(), a) == 0;
   }
@@ -608,7 +611,7 @@ struct MallocedBuffer {
   }
 
   void Truncate(size_t new_size) {
-    CHECK(new_size <= size);
+    CHECK_LE(new_size, size);
     size = new_size;
   }
 
@@ -617,7 +620,7 @@ struct MallocedBuffer {
     data = UncheckedRealloc(data, new_size);
   }
 
-  inline bool is_empty() const { return data == nullptr; }
+  bool is_empty() const { return data == nullptr; }
 
   MallocedBuffer() : data(nullptr), size(0) {}
   explicit MallocedBuffer(size_t size) : data(Malloc<T>(size)), size(size) {}