From 19b5d0750c5049986ff74ab26d249640e67ca3bf Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 20 Feb 2023 23:01:45 +0100 Subject: [PATCH] src: use string_view for report and related code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/46723 Reviewed-By: Tobias Nießen Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell Reviewed-By: Darshan Sen Reviewed-By: Chengzhong Wu --- src/json_utils.cc | 4 +-- src/json_utils.h | 25 +++++++++++----- src/node_api.cc | 2 +- src/node_errors.cc | 6 ++-- src/node_report.cc | 28 +++++++++--------- src/node_report_utils.cc | 62 +++++++++++++++++++--------------------- src/util.h | 13 +++++---- 7 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/json_utils.cc b/src/json_utils.cc index aa667ccc90471b..f387985bccd761 100644 --- 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 index 06033aa0a9b16d..06d4a7ac0905f9 100644 --- a/src/json_utils.h +++ b/src/json_utils.h @@ -4,13 +4,21 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include -#include #include +#include #include +#include 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 index 1180188553d9dd..4d95d518286b87 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -657,7 +657,7 @@ void napi_module_register_by_symbol(v8::Local 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 index f7b9551e16589c..cf6284dcfb91ca 100644 --- 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() : ""); + FPrintF( + stderr, + "%s\n", + *message ? message.ToStringView() : ""); } 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 index 2998bd09b049fd..f6439623de01e4 100644 --- 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 versions_array[NODE_VERSIONS_KEYS(V)]; + std::pair + 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( \ + #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 index b8f32beb203f6d..516eac22dc63a2 100644 --- a/src/node_report_utils.cc +++ b/src/node_report_utils.cc @@ -83,36 +83,33 @@ 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(h); - MallocedBuffer buffer(0); - size_t buffer_size = 0; + MaybeStackBuffer 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(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(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); } @@ -120,42 +117,41 @@ static void ReportPipeEndpoints(uv_handle_t* h, JSONWriter* writer) { // Utility function to format libuv path information. static void ReportPath(uv_handle_t* h, JSONWriter* writer) { - MallocedBuffer buffer(0); + MaybeStackBuffer buffer; int rc = -1; - size_t size = 0; + size_t size = buffer.capacity(); uv_any_handle* handle = reinterpret_cast(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(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 index d0e4bd41ec92c8..1d3c480b234c4c 100644 --- a/src/util.h +++ b/src/util.h @@ -38,10 +38,10 @@ #include #include #include +#include #include #include #include -#include #include #include #include @@ -486,6 +486,11 @@ class MaybeStackBuffer { free(buf_); } + inline std::basic_string ToString() const { return {out(), length()}; } + inline std::basic_string_view ToStringView() const { + return {out(), length()}; + } + private: size_t length_; // capacity of the malloc'ed buf_ @@ -533,8 +538,6 @@ class Utf8Value : public MaybeStackBuffer { public: explicit Utf8Value(v8::Isolate* isolate, v8::Local value); - inline std::string ToString() const { return std::string(out(), length()); } - inline bool operator==(const char* a) const { return strcmp(out(), a) == 0; } @@ -609,7 +612,7 @@ struct MallocedBuffer { } void Truncate(size_t new_size) { - CHECK(new_size <= size); + CHECK_LE(new_size, size); size = new_size; } @@ -618,7 +621,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(size)), size(size) {}