From 294d2ea71d5955247b96db6d814d5080cd91d36f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 2 May 2019 20:41:54 +0800 Subject: [PATCH] src: refactor V8ProfilerConnection::DispatchMessage() - Auto-generate the message id and return it for future use (we can always parse the response to find the message containing the profile instead of relying on the inspector connection being synchornous). - Generate the message from method and parameter strings and create a `StringView` directly to avoid the unnecessary copy in `ToProtocolString()`. PR-URL: https://github.com/nodejs/node/pull/27535 Reviewed-By: Jan Krems Reviewed-By: Anna Henningsen Reviewed-By: Franziska Hinkelmann Reviewed-By: Rich Trott --- src/inspector_profiler.cc | 74 +++++++++++++++------------------------ src/inspector_profiler.h | 10 +++++- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc index 530636f4d0927e..a3739d52b21de7 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -1,4 +1,5 @@ #include "inspector_profiler.h" +#include #include "base_object-inl.h" #include "debug_utils.h" #include "node_file.h" @@ -33,12 +34,6 @@ const char* const kPathSeparator = "/"; #define CWD_BUFSIZE (PATH_MAX) #endif -std::unique_ptr ToProtocolString(Isolate* isolate, - Local value) { - TwoByteValue buffer(isolate, value); - return StringBuffer::create(StringView(*buffer, buffer.length())); -} - V8ProfilerConnection::V8ProfilerConnection(Environment* env) : session_(env->inspector_agent()->Connect( std::make_unique( @@ -46,8 +41,27 @@ V8ProfilerConnection::V8ProfilerConnection(Environment* env) false)), env_(env) {} -void V8ProfilerConnection::DispatchMessage(Local message) { - session_->Dispatch(ToProtocolString(env()->isolate(), message)->string()); +size_t V8ProfilerConnection::DispatchMessage(const char* method, + const char* params) { + std::stringstream ss; + size_t id = next_id(); + ss << R"({ "id": )" << id; + DCHECK(method != nullptr); + ss << R"(, "method": ")" << method << '"'; + if (params != nullptr) { + ss << R"(, "params": )" << params; + } + ss << " }"; + std::string message = ss.str(); + const uint8_t* message_data = + reinterpret_cast(message.c_str()); + Debug(env(), + DebugCategory::INSPECTOR_PROFILER, + "Dispatching message %s\n", + message.c_str()); + session_->Dispatch(StringView(message_data, message.length())); + // TODO(joyeecheung): use this to identify the ending message. + return id; } static void WriteResult(Environment* env, @@ -202,34 +216,15 @@ std::string V8CoverageConnection::GetDirectory() const { } void V8CoverageConnection::Start() { - Debug(env(), - DebugCategory::INSPECTOR_PROFILER, - "Sending Profiler.startPreciseCoverage\n"); - Isolate* isolate = env()->isolate(); - Local enable = FIXED_ONE_BYTE_STRING( - isolate, R"({"id": 1, "method": "Profiler.enable"})"); - Local start = FIXED_ONE_BYTE_STRING(isolate, R"({ - "id": 2, - "method": "Profiler.startPreciseCoverage", - "params": { "callCount": true, "detailed": true } - })"); - DispatchMessage(enable); - DispatchMessage(start); + DispatchMessage("Profiler.enable"); + DispatchMessage("Profiler.startPreciseCoverage", + R"({ "callCount": true, "detailed": true })"); } void V8CoverageConnection::End() { CHECK_EQ(ending_, false); ending_ = true; - Debug(env(), - DebugCategory::INSPECTOR_PROFILER, - "Sending Profiler.takePreciseCoverage\n"); - Isolate* isolate = env()->isolate(); - HandleScope scope(isolate); - Local end = FIXED_ONE_BYTE_STRING(isolate, R"({ - "id": 3, - "method": "Profiler.takePreciseCoverage" - })"); - DispatchMessage(end); + DispatchMessage("Profiler.takePreciseCoverage"); } std::string V8CpuProfilerConnection::GetDirectory() const { @@ -257,25 +252,14 @@ MaybeLocal V8CpuProfilerConnection::GetProfile(Local result) { } void V8CpuProfilerConnection::Start() { - Debug(env(), DebugCategory::INSPECTOR_PROFILER, "Sending Profiler.start\n"); - Isolate* isolate = env()->isolate(); - Local enable = FIXED_ONE_BYTE_STRING( - isolate, R"({"id": 1, "method": "Profiler.enable"})"); - Local start = FIXED_ONE_BYTE_STRING( - isolate, R"({"id": 2, "method": "Profiler.start"})"); - DispatchMessage(enable); - DispatchMessage(start); + DispatchMessage("Profiler.enable"); + DispatchMessage("Profiler.start"); } void V8CpuProfilerConnection::End() { CHECK_EQ(ending_, false); ending_ = true; - Debug(env(), DebugCategory::INSPECTOR_PROFILER, "Sending Profiler.stop\n"); - Isolate* isolate = env()->isolate(); - HandleScope scope(isolate); - Local end = - FIXED_ONE_BYTE_STRING(isolate, R"({"id": 3, "method": "Profiler.stop"})"); - DispatchMessage(end); + DispatchMessage("Profiler.stop"); } // For now, we only support coverage profiling, but we may add more diff --git a/src/inspector_profiler.h b/src/inspector_profiler.h index cbe053e578cf8c..219405b8c7f3f4 100644 --- a/src/inspector_profiler.h +++ b/src/inspector_profiler.h @@ -34,7 +34,13 @@ class V8ProfilerConnection { virtual ~V8ProfilerConnection() = default; Environment* env() const { return env_; } - void DispatchMessage(v8::Local message); + + // Dispatch a protocol message, and returns the id of the message. + // `method` does not need to be surrounded by quotes. + // The optional `params` should be formatted in JSON. + // The strings should be in one byte characters - which is enough for + // the commands we use here. + size_t DispatchMessage(const char* method, const char* params = nullptr); // Use DispatchMessage() to dispatch necessary inspector messages // to start and end the profiling. @@ -55,9 +61,11 @@ class V8ProfilerConnection { v8::Local result) = 0; private: + size_t next_id() { return id_++; } void WriteProfile(v8::Local message); std::unique_ptr session_; Environment* env_ = nullptr; + size_t id_ = 1; }; class V8CoverageConnection : public V8ProfilerConnection {