From 39bf441b3b6ee5cdefb3da98861632928c821ca7 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 19 Apr 2024 11:07:36 -0400 Subject: [PATCH] build: enable Perfetto in Chromium (#41880) * build: enable perfetto in Chromium * refactor: delete TracingControllerImpl * fix: TraceObject isn't present when v8_use_perfetto is true * fix: update lib/internal/http for perfetto * chore: remove stray log --- build/args/all.gn | 4 - patches/node/.patches | 1 + patches/node/build_enable_perfetto.patch | 370 +++++++++++++++++++++++ shell/browser/javascript_environment.cc | 149 +-------- 4 files changed, 372 insertions(+), 152 deletions(-) create mode 100644 patches/node/build_enable_perfetto.patch diff --git a/build/args/all.gn b/build/args/all.gn index 6a8d0d386302..d75e30679d34 100644 --- a/build/args/all.gn +++ b/build/args/all.gn @@ -50,10 +50,6 @@ is_cfi = false # TODO: fix this once sysroots have been updated. use_qt = false -# https://chromium-review.googlesource.com/c/chromium/src/+/4365718 -# TODO(codebytere): fix perfetto incompatibility with Node.js. -use_perfetto_client_library = false - # Disables the builtins PGO for V8 v8_builtins_profiling_log_file = "" diff --git a/patches/node/.patches b/patches/node/.patches index 17b0aa74e97d..f3660a139a5a 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -45,3 +45,4 @@ fix_revert_src_lb_reducing_c_calls_of_esm_legacy_main_resolve.patch src_preload_function_for_environment.patch deprecate_vector_v8_local_in_v8.patch fix_remove_deprecated_errno_constants.patch +build_enable_perfetto.patch diff --git a/patches/node/build_enable_perfetto.patch b/patches/node/build_enable_perfetto.patch new file mode 100644 index 000000000000..d2a52425532d --- /dev/null +++ b/patches/node/build_enable_perfetto.patch @@ -0,0 +1,370 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shelley Vohr +Date: Wed, 17 Apr 2024 08:17:49 -0400 +Subject: build: enable perfetto + +Enable perfetto by default in Node.js. Node.js disables perfetto by +default but is broken on build - they don't currently add guards for +`V8_USE_PERFETTO` and upstream only defines certain functions +on `v8::TracingController` if perfetto is disabled. Electron already +had minimal to no support for Node.js trace events, so the impact of +adding associated guards there should be relatively small. + +We should upstream this as it will eventually impact Node.js as well. + +diff --git a/lib/internal/constants.js b/lib/internal/constants.js +index 8d7204f6cb48f783adc4d1c1eb2de0c83b7fffe2..a154559a56bf383d3c26af523c9bb07b564ef600 100644 +--- a/lib/internal/constants.js ++++ b/lib/internal/constants.js +@@ -5,12 +5,15 @@ const isWindows = process.platform === 'win32'; + module.exports = { + // Alphabet chars. + CHAR_UPPERCASE_A: 65, /* A */ ++ CHAR_UPPERCASE_B: 66, /* B */ + CHAR_LOWERCASE_A: 97, /* a */ + CHAR_UPPERCASE_Z: 90, /* Z */ + CHAR_LOWERCASE_Z: 122, /* z */ + CHAR_UPPERCASE_C: 67, /* C */ + CHAR_LOWERCASE_B: 98, /* b */ ++ CHAR_UPPERCASE_E: 69, /* E */ + CHAR_LOWERCASE_E: 101, /* e */ ++ + CHAR_LOWERCASE_N: 110, /* n */ + + // Non-alphabetic chars. +diff --git a/lib/internal/http.js b/lib/internal/http.js +index b20b3cd229efcd9701791309361b7d106f315900..6b2820c9dcce5e658b694f53e75d93707c4320d7 100644 +--- a/lib/internal/http.js ++++ b/lib/internal/http.js +@@ -10,8 +10,8 @@ const { + const { setUnrefTimeout } = require('internal/timers'); + const { trace, isTraceCategoryEnabled } = internalBinding('trace_events'); + const { +- CHAR_LOWERCASE_B, +- CHAR_LOWERCASE_E, ++ CHAR_UPPERCASE_B, ++ CHAR_UPPERCASE_E, + } = require('internal/constants'); + + let utcCache; +@@ -44,11 +44,13 @@ function isTraceHTTPEnabled() { + const traceEventCategory = 'node,node.http'; + + function traceBegin(...args) { +- trace(CHAR_LOWERCASE_B, traceEventCategory, ...args); ++ // See v8/src/builtins/builtins-trace.cc - must be uppercase for perfetto ++ trace(CHAR_UPPERCASE_B, traceEventCategory, ...args); + } + + function traceEnd(...args) { +- trace(CHAR_LOWERCASE_E, traceEventCategory, ...args); ++ // See v8/src/builtins/builtins-trace.cc - must be uppercase for perfetto ++ trace(CHAR_UPPERCASE_E, traceEventCategory, ...args); + } + + module.exports = { +diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc +index 7ce59674356f9743438350949be42fa7ead2afbe..c5fedc3be86a77730c57321b9c73cc8e94a001d7 100644 +--- a/src/tracing/agent.cc ++++ b/src/tracing/agent.cc +@@ -50,7 +50,9 @@ using v8::platform::tracing::TraceWriter; + using std::string; + + Agent::Agent() : tracing_controller_(new TracingController()) { ++#ifndef V8_USE_PERFETTO + tracing_controller_->Initialize(nullptr); ++#endif + + CHECK_EQ(uv_loop_init(&tracing_loop_), 0); + CHECK_EQ(uv_async_init(&tracing_loop_, +@@ -86,10 +88,14 @@ Agent::~Agent() { + void Agent::Start() { + if (started_) + return; +- ++#ifdef V8_USE_PERFETTO ++ std::ostringstream perfetto_output; ++ tracing_controller_->InitializeForPerfetto(&perfetto_output); ++#else + NodeTraceBuffer* trace_buffer_ = new NodeTraceBuffer( + NodeTraceBuffer::kBufferChunks, this, &tracing_loop_); + tracing_controller_->Initialize(trace_buffer_); ++#endif + + // This thread should be created *after* async handles are created + // (within NodeTraceWriter and NodeTraceBuffer constructors). +@@ -143,8 +149,10 @@ void Agent::StopTracing() { + return; + // Perform final Flush on TraceBuffer. We don't want the tracing controller + // to flush the buffer again on destruction of the V8::Platform. +- tracing_controller_->StopTracing(); ++#ifndef V8_USE_PERFETTO + tracing_controller_->Initialize(nullptr); ++#endif ++ tracing_controller_->StopTracing(); + started_ = false; + + // Thread should finish when the tracing loop is stopped. +@@ -202,6 +210,7 @@ std::string Agent::GetEnabledCategories() const { + return categories; + } + ++#ifndef V8_USE_PERFETTO + void Agent::AppendTraceEvent(TraceObject* trace_event) { + for (const auto& id_writer : writers_) + id_writer.second->AppendTraceEvent(trace_event); +@@ -211,18 +220,21 @@ void Agent::AddMetadataEvent(std::unique_ptr event) { + Mutex::ScopedLock lock(metadata_events_mutex_); + metadata_events_.push_back(std::move(event)); + } ++#endif + + void Agent::Flush(bool blocking) { ++#ifndef V8_USE_PERFETTO + { + Mutex::ScopedLock lock(metadata_events_mutex_); + for (const auto& event : metadata_events_) + AppendTraceEvent(event.get()); + } +- ++#endif + for (const auto& id_writer : writers_) + id_writer.second->Flush(blocking); + } + ++#ifndef V8_USE_PERFETTO + void TracingController::AddMetadataEvent( + const unsigned char* category_group_enabled, + const char* name, +@@ -246,6 +258,6 @@ void TracingController::AddMetadataEvent( + if (node_agent != nullptr) + node_agent->AddMetadataEvent(std::move(trace_event)); + } +- ++#endif + } // namespace tracing + } // namespace node +diff --git a/src/tracing/agent.h b/src/tracing/agent.h +index b542a849fe8da7e8bbbcca7067b73dc32b18d6d3..059ce6f6ea17199ead09c6c13bcc680f18f8c4d0 100644 +--- a/src/tracing/agent.h ++++ b/src/tracing/agent.h +@@ -27,7 +27,9 @@ class Agent; + class AsyncTraceWriter { + public: + virtual ~AsyncTraceWriter() = default; ++#ifndef V8_USE_PERFETTO + virtual void AppendTraceEvent(TraceObject* trace_event) = 0; ++#endif + virtual void Flush(bool blocking) = 0; + virtual void InitializeOnThread(uv_loop_t* loop) {} + }; +@@ -36,6 +38,7 @@ class TracingController : public v8::platform::tracing::TracingController { + public: + TracingController() : v8::platform::tracing::TracingController() {} + ++#ifndef V8_USE_PERFETTO + int64_t CurrentTimestampMicroseconds() override { + return uv_hrtime() / 1000; + } +@@ -48,6 +51,7 @@ class TracingController : public v8::platform::tracing::TracingController { + const uint64_t* arg_values, + std::unique_ptr* convertable_values, + unsigned int flags); ++#endif + }; + + class AgentWriterHandle { +@@ -108,11 +112,12 @@ class Agent { + + // Returns a comma-separated list of enabled categories. + std::string GetEnabledCategories() const; +- ++#ifndef V8_USE_PERFETTO + // Writes to all writers registered through AddClient(). + void AppendTraceEvent(TraceObject* trace_event); + + void AddMetadataEvent(std::unique_ptr event); ++#endif + // Flushes all writers registered through AddClient(). + void Flush(bool blocking); + +@@ -152,7 +157,9 @@ class Agent { + std::set to_be_initialized_; + + Mutex metadata_events_mutex_; ++#ifndef V8_USE_PERFETTO + std::list> metadata_events_; ++#endif + }; + + void AgentWriterHandle::reset() { +diff --git a/src/tracing/node_trace_buffer.cc b/src/tracing/node_trace_buffer.cc +index e187a1d78c81972b69cd4e03f7079cdb727956ad..3256c6326a08c6cafd83f1e49e3350193e813b51 100644 +--- a/src/tracing/node_trace_buffer.cc ++++ b/src/tracing/node_trace_buffer.cc +@@ -55,6 +55,7 @@ TraceObject* InternalTraceBuffer::GetEventByHandle(uint64_t handle) { + } + + void InternalTraceBuffer::Flush(bool blocking) { ++#ifndef V8_USE_PERFETTO + { + Mutex::ScopedLock scoped_lock(mutex_); + if (total_chunks_ > 0) { +@@ -75,6 +76,7 @@ void InternalTraceBuffer::Flush(bool blocking) { + flushing_ = false; + } + } ++#endif + agent_->Flush(blocking); + } + +diff --git a/src/tracing/node_trace_writer.cc b/src/tracing/node_trace_writer.cc +index 8f053efe93324b9acbb4e85f7b974b4f7712e200..e331ed5567caa39ade90ce28cea69f1d10533812 100644 +--- a/src/tracing/node_trace_writer.cc ++++ b/src/tracing/node_trace_writer.cc +@@ -95,7 +95,7 @@ void NodeTraceWriter::OpenNewFileForStreaming() { + fd_ = -1; + } + } +- ++#ifndef V8_USE_PERFETTO + void NodeTraceWriter::AppendTraceEvent(TraceObject* trace_event) { + Mutex::ScopedLock scoped_lock(stream_mutex_); + // If this is the first trace event, open a new file for streaming. +@@ -112,7 +112,7 @@ void NodeTraceWriter::AppendTraceEvent(TraceObject* trace_event) { + ++total_traces_; + json_trace_writer_->AppendTraceEvent(trace_event); + } +- ++#endif + void NodeTraceWriter::FlushPrivate() { + std::string str; + int highest_request_id; +diff --git a/src/tracing/node_trace_writer.h b/src/tracing/node_trace_writer.h +index cd965d77b7859ff2edcf781a934594b5a9b6d251..fe1714ba77fddef693d37eeb8c7a196ddfd15c26 100644 +--- a/src/tracing/node_trace_writer.h ++++ b/src/tracing/node_trace_writer.h +@@ -20,7 +20,9 @@ class NodeTraceWriter : public AsyncTraceWriter { + ~NodeTraceWriter() override; + + void InitializeOnThread(uv_loop_t* loop) override; ++#ifndef V8_USE_PERFETTO + void AppendTraceEvent(TraceObject* trace_event) override; ++#endif + void Flush(bool blocking) override; + + static const int kTracesPerFile = 1 << 19; +diff --git a/src/tracing/trace_event.h b/src/tracing/trace_event.h +index be0f55a409a71bf9c1763c36fdc252857228742e..827b5330b2f8c545338a46c548f8abf4aab7f50c 100644 +--- a/src/tracing/trace_event.h ++++ b/src/tracing/trace_event.h +@@ -69,8 +69,16 @@ enum CategoryGroupEnabledFlags { + // for best performance when tracing is disabled. + // const uint8_t* + // TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(const char* category_group) ++#ifndef V8_USE_PERFETTO + #define TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED \ + node::tracing::TraceEventHelper::GetCategoryGroupEnabled ++#else ++#define TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(category_group) \ ++ ([](const char*) -> const uint8_t* { \ ++ static uint8_t no = 0; \ ++ return &no; \ ++ })(category_group) ++#endif + + // Get the number of times traces have been recorded. This is used to implement + // the TRACE_EVENT_IS_NEW_TRACE facility. +@@ -114,10 +122,15 @@ enum CategoryGroupEnabledFlags { + // const uint8_t* category_group_enabled, + // const char* name, + // uint64_t id) ++#ifndef V8_USE_PERFETTO + #define TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION \ + if (auto controller = \ + node::tracing::TraceEventHelper::GetTracingController()) \ + controller->UpdateTraceEventDuration ++#else ++#define TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION(category_group_enabled, name, event_handle) \ ++ (void)(category_group_enabled), (void)(name), (void)(event_handle) ++#endif + + // Adds a metadata event to the trace log. The |AppendValueAsTraceFormat| method + // on the convertable value will be called at flush time. +@@ -319,10 +332,13 @@ class TraceEventHelper { + static void SetAgent(Agent* agent); + + static inline const uint8_t* GetCategoryGroupEnabled(const char* group) { ++#ifndef V8_USE_PERFETTO + v8::TracingController* controller = GetTracingController(); + static const uint8_t disabled = 0; + if (UNLIKELY(controller == nullptr)) return &disabled; + return controller->GetCategoryGroupEnabled(group); ++#endif ++ return 0; + } + }; + +@@ -460,6 +476,7 @@ static inline uint64_t AddTraceEventImpl( + const char* scope, uint64_t id, uint64_t bind_id, int32_t num_args, + const char** arg_names, const uint8_t* arg_types, + const uint64_t* arg_values, unsigned int flags) { ++#ifndef V8_USE_PERFETTO + std::unique_ptr arg_convertibles[2]; + if (num_args > 0 && arg_types[0] == TRACE_VALUE_TYPE_CONVERTABLE) { + arg_convertibles[0].reset(reinterpret_cast( +@@ -469,13 +486,14 @@ static inline uint64_t AddTraceEventImpl( + arg_convertibles[1].reset(reinterpret_cast( + static_cast(arg_values[1]))); + } +- // DCHECK(num_args, 2); + v8::TracingController* controller = + node::tracing::TraceEventHelper::GetTracingController(); + if (controller == nullptr) return 0; + return controller->AddTraceEvent(phase, category_group_enabled, name, scope, id, + bind_id, num_args, arg_names, arg_types, + arg_values, arg_convertibles, flags); ++#endif ++ return 0; + } + + static V8_INLINE uint64_t AddTraceEventWithTimestampImpl( +@@ -483,6 +501,7 @@ static V8_INLINE uint64_t AddTraceEventWithTimestampImpl( + const char* scope, uint64_t id, uint64_t bind_id, int32_t num_args, + const char** arg_names, const uint8_t* arg_types, + const uint64_t* arg_values, unsigned int flags, int64_t timestamp) { ++#ifndef V8_USE_PERFETTO + std::unique_ptr arg_convertables[2]; + if (num_args > 0 && arg_types[0] == TRACE_VALUE_TYPE_CONVERTABLE) { + arg_convertables[0].reset(reinterpret_cast( +@@ -492,19 +511,21 @@ static V8_INLINE uint64_t AddTraceEventWithTimestampImpl( + arg_convertables[1].reset(reinterpret_cast( + static_cast(arg_values[1]))); + } +- // DCHECK_LE(num_args, 2); + v8::TracingController* controller = + node::tracing::TraceEventHelper::GetTracingController(); + if (controller == nullptr) return 0; + return controller->AddTraceEventWithTimestamp( + phase, category_group_enabled, name, scope, id, bind_id, num_args, + arg_names, arg_types, arg_values, arg_convertables, flags, timestamp); ++#endif ++ return 0; + } + + static V8_INLINE void AddMetadataEventImpl( + const uint8_t* category_group_enabled, const char* name, int32_t num_args, + const char** arg_names, const uint8_t* arg_types, + const uint64_t* arg_values, unsigned int flags) { ++#ifndef V8_USE_PERFETTO + std::unique_ptr arg_convertibles[2]; + if (num_args > 0 && arg_types[0] == TRACE_VALUE_TYPE_CONVERTABLE) { + arg_convertibles[0].reset(reinterpret_cast( +@@ -520,6 +541,7 @@ static V8_INLINE void AddMetadataEventImpl( + return agent->GetTracingController()->AddMetadataEvent( + category_group_enabled, name, num_args, arg_names, arg_types, arg_values, + arg_convertibles, flags); ++#endif + } + + // Define SetTraceValue for each allowed type. It stores the type and diff --git a/shell/browser/javascript_environment.cc b/shell/browser/javascript_environment.cc index f91ac8a38984..05ea280b0f74 100644 --- a/shell/browser/javascript_environment.cc +++ b/shell/browser/javascript_environment.cc @@ -131,153 +131,6 @@ JavascriptEnvironment::~JavascriptEnvironment() { platform_->UnregisterIsolate(isolate_); } -class EnabledStateObserverImpl final - : public base::trace_event::TraceLog::EnabledStateObserver { - public: - EnabledStateObserverImpl() { - base::trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(this); - } - - ~EnabledStateObserverImpl() override { - base::trace_event::TraceLog::GetInstance()->RemoveEnabledStateObserver( - this); - } - - // disable copy - EnabledStateObserverImpl(const EnabledStateObserverImpl&) = delete; - EnabledStateObserverImpl& operator=(const EnabledStateObserverImpl&) = delete; - - void OnTraceLogEnabled() final { - base::AutoLock lock(mutex_); - for (auto* o : observers_) { - o->OnTraceEnabled(); - } - } - - void OnTraceLogDisabled() final { - base::AutoLock lock(mutex_); - for (auto* o : observers_) { - o->OnTraceDisabled(); - } - } - - void AddObserver(v8::TracingController::TraceStateObserver* observer) { - { - base::AutoLock lock(mutex_); - DCHECK(!observers_.count(observer)); - observers_.insert(observer); - } - - // Fire the observer if recording is already in progress. - if (base::trace_event::TraceLog::GetInstance()->IsEnabled()) - observer->OnTraceEnabled(); - } - - void RemoveObserver(v8::TracingController::TraceStateObserver* observer) { - base::AutoLock lock(mutex_); - DCHECK_EQ(observers_.count(observer), 1lu); - observers_.erase(observer); - } - - private: - base::Lock mutex_; - std::unordered_set observers_; -}; - -class TracingControllerImpl : public node::tracing::TracingController { - public: - TracingControllerImpl() = default; - ~TracingControllerImpl() override = default; - - // disable copy - TracingControllerImpl(const TracingControllerImpl&) = delete; - TracingControllerImpl& operator=(const TracingControllerImpl&) = delete; - - // TracingController implementation. - const uint8_t* GetCategoryGroupEnabled(const char* name) override { - return TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(name); - } - uint64_t AddTraceEvent( - char phase, - const uint8_t* category_enabled_flag, - const char* name, - const char* scope, - uint64_t id, - uint64_t bind_id, - int32_t num_args, - const char** arg_names, - const uint8_t* arg_types, - const uint64_t* arg_values, - std::unique_ptr* arg_convertables, - unsigned int flags) override { - base::trace_event::TraceArguments args( - num_args, arg_names, arg_types, - reinterpret_cast( // NOLINT(runtime/int) - arg_values), - arg_convertables); - DCHECK_LE(num_args, 2); - base::trace_event::TraceEventHandle handle = - TRACE_EVENT_API_ADD_TRACE_EVENT_WITH_BIND_ID( - phase, category_enabled_flag, name, scope, id, bind_id, &args, - flags); - uint64_t result; - memcpy(&result, &handle, sizeof(result)); - return result; - } - uint64_t AddTraceEventWithTimestamp( - char phase, - const uint8_t* category_enabled_flag, - const char* name, - const char* scope, - uint64_t id, - uint64_t bind_id, - int32_t num_args, - const char** arg_names, - const uint8_t* arg_types, - const uint64_t* arg_values, - std::unique_ptr* arg_convertables, - unsigned int flags, - int64_t timestampMicroseconds) override { - base::trace_event::TraceArguments args( - num_args, arg_names, arg_types, - reinterpret_cast( // NOLINT(runtime/int) - arg_values), - arg_convertables); - DCHECK_LE(num_args, 2); - base::TimeTicks timestamp = - base::TimeTicks() + base::Microseconds(timestampMicroseconds); - base::trace_event::TraceEventHandle handle = - TRACE_EVENT_API_ADD_TRACE_EVENT_WITH_THREAD_ID_AND_TIMESTAMP( - phase, category_enabled_flag, name, scope, id, bind_id, - TRACE_EVENT_API_CURRENT_THREAD_ID, timestamp, &args, flags); - uint64_t result; - memcpy(&result, &handle, sizeof(result)); - return result; - } - void UpdateTraceEventDuration(const uint8_t* category_enabled_flag, - const char* name, - uint64_t handle) override { - base::trace_event::TraceEventHandle traceEventHandle; - memcpy(&traceEventHandle, &handle, sizeof(handle)); - TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION(category_enabled_flag, name, - traceEventHandle); - } - - void AddTraceStateObserver(TraceStateObserver* observer) override { - GetObserverDelegate().AddObserver(observer); - } - - void RemoveTraceStateObserver(TraceStateObserver* observer) override { - GetObserverDelegate().RemoveObserver(observer); - } - - private: - static EnabledStateObserverImpl& GetObserverDelegate() { - static base::NoDestructor instance; - return *instance; - } -}; - v8::Isolate* JavascriptEnvironment::Initialize(uv_loop_t* event_loop, bool setup_wasm_streaming) { auto* cmd = base::CommandLine::ForCurrentProcess(); @@ -292,7 +145,7 @@ v8::Isolate* JavascriptEnvironment::Initialize(uv_loop_t* event_loop, // The V8Platform of gin relies on Chromium's task schedule, which has not // been started at this point, so we have to rely on Node's V8Platform. auto* tracing_agent = node::CreateAgent(); - auto* tracing_controller = new TracingControllerImpl(); + auto* tracing_controller = tracing_agent->GetTracingController(); node::tracing::TraceEventHelper::SetAgent(tracing_agent); platform_ = node::MultiIsolatePlatform::Create( base::RecommendedMaxNumberOfThreadsInThreadGroup(3, 8, 0.1, 0),