From 8b33c87239c35f665c39ad1c4030e06069e4e277 Mon Sep 17 00:00:00 2001 From: Sara Tang Date: Tue, 5 Jan 2021 08:43:34 -1000 Subject: [PATCH] Step 1 (of 3-ish): Basic ETW Instrumentation in V8 Design doc: https://docs.google.com/document/d/1xkXj94iExFgLWc_OszTNyNGi523ARaKMWPZTeomhI4U A lot has changed since the last patchset! I recommend revisiting this design doc and reading the parts in green. I explain the roadmap for what changes to expect from ETW instrumentation as well as the instrumentation of this particular CL. I'll do my best to answer any further questions anyone has about my particular instrumentation or ETW in general :) --- This is the first of a series of changelists to round out ETW instrumentation for V8. This changelist represents the most minimal change needed to instrument ETW in V8. In particular, it: - defines and registers the ETW provider, - interacts minimally with the rest of V8, by hooking into the existing TracingController::AddTraceEvent function, - is designed with a platform-agnostic layer, so that event tracers for other platforms can be instrumented in teh future. Some notes on instrumentation (aka I copied stuff from the design doc): We make heavy use of the TraceLogging API to log events. It differs from previous methods of emitting ETW events in that it doesn<80><99>t require the overhead of a separate manifest file to keep track of metadata; rather, events using this API are self-descriptive. Here are the five major steps to instrument the TraceLogging API: - Forward declare the provider (from provider-win.h) - Define the provider in a .cc file (from provider-win.cc) - Register the provider (called from v8.cc). - Write events (called from libplatform/tracing-controller.cc) - Unregister the provider (called from v8.cc) At the base, we have an abstract provider class that encapsulates the functionality of an event provider. These are things like registering and unregistering the provider, and the actual event-logging. The provider class is split into provider-win and provider-mac (currently not instantiated) classes, with OS-dependent implementations of the above functions. In particular, the TraceLogging API is used only in provider-win. It is here that we forward declare and define the provider, as well as write ETW events. Finally, there is a v8-provider class that serves as a top-level API and is exposed to the rest of V8. It acts as a wrapper for the platform-specific providers. The .wprp file is needed so that Windows Performance Recorder knows how to capture our events. Some considerations: - Is TracingController::AddTraceEvent the best place from which to write my events? - Is src/libplatform/tracing the best place to put my instrumentation? - Right now, I fail the preupload because of this, which tells me my files are probably not in the best location: You added one or more #includes that violate checkdeps rules. src\init\v8.cc Illegal include: "src/libplatform/tracing/v8-provider.h" Because of "-src/libplatform" from src's include_rules. Change-Id: Id53e4a034c9e526524a17000da0a647a95d93edf Bug: v8:11043 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2233407 Reviewed-by: Ulan Degenbaev Reviewed-by: Peter Marshall Commit-Queue: Sara Tang Cr-Commit-Position: refs/heads/master@{#71918} --- BUILD.gn | 22 +++++++ include/libplatform/v8-tracing.h | 2 + src/d8/d8.cc | 32 +++++++-- src/d8/d8.h | 2 + src/libplatform/tracing/recorder-default.cc | 25 +++++++ src/libplatform/tracing/recorder-win.cc | 72 +++++++++++++++++++++ src/libplatform/tracing/recorder.h | 37 +++++++++++ src/libplatform/tracing/trace-writer.cc | 22 +++++++ src/libplatform/tracing/trace-writer.h | 13 ++++ tools/wpr.wprp | 68 +++++++++++++++++++ 10 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 src/libplatform/tracing/recorder-default.cc create mode 100644 src/libplatform/tracing/recorder-win.cc create mode 100644 src/libplatform/tracing/recorder.h create mode 100644 tools/wpr.wprp diff --git a/BUILD.gn b/BUILD.gn index bc4e695b3a..99071a4f1d 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -45,6 +45,12 @@ declare_args() { # Sets --DV8_LITE_MODE. v8_enable_lite_mode = false + # Sets -DSYSTEM_INSTRUMENTATION. Enables OS-dependent event tracing + v8_enable_system_instrumentation = false + + # Sets the GUID for the ETW provider + v8_etw_guid = "" + # Sets -DVERIFY_HEAP. v8_enable_verify_heap = "" @@ -695,6 +701,12 @@ config("features") { if (v8_dict_mode_prototypes) { defines += [ "V8_DICT_MODE_PROTOTYPES" ] } + if (v8_enable_system_instrumentation) { + defines += [ "V8_ENABLE_SYSTEM_INSTRUMENTATION" ] + } + if (v8_etw_guid != "") { + defines += [ "V8_ETW_GUID=\"$v8_etw_guid\"" ] + } } config("toolchain") { @@ -4262,6 +4274,10 @@ v8_component("v8_libbase") { "ws2_32.lib", ] + if (v8_enable_system_instrumentation) { + libs += [ "advapi32.lib" ] # Needed for TraceLoggingProvider.h + } + data_deps += [ "//build/win:runtime_libs" ] } @@ -4304,6 +4320,8 @@ v8_component("v8_libplatform") { "src/libplatform/delayed-task-queue.h", "src/libplatform/task-queue.cc", "src/libplatform/task-queue.h", + "src/libplatform/tracing/recorder-default.cc", + "src/libplatform/tracing/recorder.h", "src/libplatform/tracing/trace-buffer.cc", "src/libplatform/tracing/trace-buffer.h", "src/libplatform/tracing/trace-config.cc", @@ -4335,6 +4353,7 @@ v8_component("v8_libplatform") { if (v8_use_perfetto) { sources -= [ "//base/trace_event/common/trace_event_common.h", + "src/libplatform/tracing/recorder-default.cc", "src/libplatform/tracing/trace-buffer.cc", "src/libplatform/tracing/trace-buffer.h", "src/libplatform/tracing/trace-object.cc", @@ -4349,6 +4368,9 @@ v8_component("v8_libplatform") { # TODO(skyostil): Switch TraceEventListener to protozero. "//third_party/perfetto/protos/perfetto/trace:lite", ] + } else if (is_win) { + sources -= [ "src/libplatform/tracing/recorder-default.cc" ] + sources += [ "src/libplatform/tracing/recorder-win.cc" ] } } diff --git a/include/libplatform/v8-tracing.h b/include/libplatform/v8-tracing.h index 45822d00f3..c7a5c4f9f5 100644 --- a/include/libplatform/v8-tracing.h +++ b/include/libplatform/v8-tracing.h @@ -125,6 +125,8 @@ class V8_PLATFORM_EXPORT TraceWriter { static TraceWriter* CreateJSONTraceWriter(std::ostream& stream, const std::string& tag); + static TraceWriter* CreateSystemInstrumentationTraceWriter(); + private: // Disallow copy and assign TraceWriter(const TraceWriter&) = delete; diff --git a/src/d8/d8.cc b/src/d8/d8.cc index 92b806823a..55746bd5dd 100644 --- a/src/d8/d8.cc +++ b/src/d8/d8.cc @@ -3645,6 +3645,12 @@ bool Shell::SetOptions(int argc, char* argv[]) { } else if (strcmp(argv[i], "--fuzzy-module-file-extensions") == 0) { options.fuzzy_module_file_extensions = true; argv[i] = nullptr; +#ifdef V8_ENABLE_SYSTEM_INSTRUMENTATION + } else if (strcmp(argv[i], "--enable-system-instrumentation") == 0) { + options.enable_system_instrumentation = true; + options.trace_enabled = true; + argv[i] = nullptr; +#endif } } @@ -4216,8 +4222,8 @@ int Shell::Main(int argc, char* argv[]) { ? v8::platform::InProcessStackDumping::kDisabled : v8::platform::InProcessStackDumping::kEnabled; - std::unique_ptr tracing; std::ofstream trace_file; + std::unique_ptr tracing; if (options.trace_enabled && !i::FLAG_verify_predictable) { tracing = std::make_unique(); const char* trace_path = @@ -4238,10 +4244,23 @@ int Shell::Main(int argc, char* argv[]) { tracing->InitializeForPerfetto(&trace_file); #else - platform::tracing::TraceBuffer* trace_buffer = - platform::tracing::TraceBuffer::CreateTraceBufferRingBuffer( - platform::tracing::TraceBuffer::kRingBufferChunks, - platform::tracing::TraceWriter::CreateJSONTraceWriter(trace_file)); + platform::tracing::TraceBuffer* trace_buffer = nullptr; +#if defined(V8_ENABLE_SYSTEM_INSTRUMENTATION) + if (options.enable_system_instrumentation) { + trace_buffer = + platform::tracing::TraceBuffer::CreateTraceBufferRingBuffer( + platform::tracing::TraceBuffer::kRingBufferChunks, + platform::tracing::TraceWriter:: + CreateSystemInstrumentationTraceWriter()); + } +#endif // V8_ENABLE_SYSTEM_INSTRUMENTATION + if (!trace_buffer) { + trace_buffer = + platform::tracing::TraceBuffer::CreateTraceBufferRingBuffer( + platform::tracing::TraceBuffer::kRingBufferChunks, + platform::tracing::TraceWriter::CreateJSONTraceWriter( + trace_file)); + } tracing->Initialize(trace_buffer); #endif // V8_USE_PERFETTO } @@ -4358,6 +4377,9 @@ int Shell::Main(int argc, char* argv[]) { } else { trace_config = platform::tracing::TraceConfig::CreateDefaultTraceConfig(); + if (options.enable_system_instrumentation) { + trace_config->AddIncludedCategory("disabled-by-default-v8.compile"); + } } tracing_controller->StartTracing(trace_config); } diff --git a/src/d8/d8.h b/src/d8/d8.h index 4c5c801c84..a443ea0f78 100644 --- a/src/d8/d8.h +++ b/src/d8/d8.h @@ -387,6 +387,8 @@ class ShellOptions { DisallowReassignment cpu_profiler_print = {"cpu-profiler-print", false}; DisallowReassignment fuzzy_module_file_extensions = { "fuzzy-module-file-extensions", true}; + DisallowReassignment enable_system_instrumentation = { + "enable-system-instrumentation", false}; }; class Shell : public i::AllStatic { diff --git a/src/libplatform/tracing/recorder-default.cc b/src/libplatform/tracing/recorder-default.cc new file mode 100644 index 0000000000..46e0cbb8e2 --- /dev/null +++ b/src/libplatform/tracing/recorder-default.cc @@ -0,0 +1,25 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#ifndef V8_LIBPLATFORM_TRACING_RECORDER_DEFAULT_H_ +#define V8_LIBPLATFORM_TRACING_RECORDER_DEFAULT_H_ + +#include "src/libplatform/tracing/recorder.h" + +namespace v8 { +namespace platform { +namespace tracing { + +Recorder::Recorder() {} +Recorder::~Recorder() {} + +bool Recorder::IsEnabled() { return false; } +bool Recorder::IsEnabled(const uint8_t level) { return false; } + +void Recorder::AddEvent(TraceObject* trace_event) {} + +} // namespace tracing +} // namespace platform +} // namespace v8 + +#endif // V8_LIBPLATFORM_TRACING_RECORDER_DEFAULT_H_ diff --git a/src/libplatform/tracing/recorder-win.cc b/src/libplatform/tracing/recorder-win.cc new file mode 100644 index 0000000000..b15704c050 --- /dev/null +++ b/src/libplatform/tracing/recorder-win.cc @@ -0,0 +1,72 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#ifndef V8_LIBPLATFORM_TRACING_RECORDER_WIN_H_ +#define V8_LIBPLATFORM_TRACING_RECORDER_WIN_H_ + +#include +#include + +#include "src/libplatform/tracing/recorder.h" + +#if defined(__clang__) +#pragma clang diagnostic ignored "-Wc++98-compat-extra-semi" +#endif + +#ifndef V8_ETW_GUID +#define V8_ETW_GUID \ + 0x57277741, 0x3638, 0x4A4B, 0xBD, 0xBA, 0x0A, 0xC6, 0xE4, 0x5D, 0xA5, 0x6C +#endif + +namespace v8 { +namespace platform { +namespace tracing { + +TRACELOGGING_DECLARE_PROVIDER(g_v8Provider); + +TRACELOGGING_DEFINE_PROVIDER(g_v8Provider, "V8.js", (V8_ETW_GUID)); + +Recorder::Recorder() { TraceLoggingRegister(g_v8Provider); } + +Recorder::~Recorder() { + if (g_v8Provider) { + TraceLoggingUnregister(g_v8Provider); + } +} + +bool Recorder::IsEnabled() { + return TraceLoggingProviderEnabled(g_v8Provider, 0, 0); +} + +bool Recorder::IsEnabled(const uint8_t level) { + return TraceLoggingProviderEnabled(g_v8Provider, level, 0); +} + +void Recorder::AddEvent(TraceObject* trace_event) { + // TODO(sartang@microsoft.com): Figure out how to write the conditional + // arguments + wchar_t* wName = new wchar_t[4096]; + MultiByteToWideChar(CP_ACP, 0, trace_event->name(), -1, wName, 4096); + + wchar_t* wCategoryGroupName = new wchar_t[4096]; + MultiByteToWideChar(CP_ACP, 0, + TracingController::GetCategoryGroupName( + trace_event->category_enabled_flag()), + -1, wCategoryGroupName, 4096); + + TraceLoggingWrite(g_v8Provider, "", TraceLoggingValue(wName, "Event Name"), + TraceLoggingValue(trace_event->pid(), "pid"), + TraceLoggingValue(trace_event->tid(), "tid"), + TraceLoggingValue(trace_event->ts(), "ts"), + TraceLoggingValue(trace_event->tts(), "tts"), + TraceLoggingValue(trace_event->phase(), "phase"), + TraceLoggingValue(wCategoryGroupName, "category"), + TraceLoggingValue(trace_event->duration(), "dur"), + TraceLoggingValue(trace_event->cpu_duration(), "tdur")); +} + +} // namespace tracing +} // namespace platform +} // namespace v8 + +#endif // V8_LIBPLATFORM_TRACING_RECORDER_WIN_H_ diff --git a/src/libplatform/tracing/recorder.h b/src/libplatform/tracing/recorder.h new file mode 100644 index 0000000000..4e14a038e8 --- /dev/null +++ b/src/libplatform/tracing/recorder.h @@ -0,0 +1,37 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef V8_LIBPLATFORM_TRACING_RECORDER_H_ +#define V8_LIBPLATFORM_TRACING_RECORDER_H_ + +#include + +#include "include/libplatform/v8-tracing.h" + +namespace v8 { +namespace platform { +namespace tracing { + +// This class serves as a base class for emitting events to system event +// controllers: ETW for Windows, Signposts on Mac (to be implemented). It is +// enabled by turning on both the ENABLE_SYSTEM_INSTRUMENTATION build flag and +// the --enable-system-instrumentation command line flag. When enabled, it is +// called from within SystemInstrumentationTraceWriter and replaces the +// JSONTraceWriter for event-tracing. +class Recorder { + public: + Recorder(); + ~Recorder(); + + bool IsEnabled(); + bool IsEnabled(const uint8_t level); + + void AddEvent(TraceObject* trace_event); +}; + +} // namespace tracing +} // namespace platform +} // namespace v8 + +#endif // V8_LIBPLATFORM_TRACING_RECORDER_H_ diff --git a/src/libplatform/tracing/trace-writer.cc b/src/libplatform/tracing/trace-writer.cc index 1d3e53ac3b..5740dabd8f 100644 --- a/src/libplatform/tracing/trace-writer.cc +++ b/src/libplatform/tracing/trace-writer.cc @@ -9,6 +9,7 @@ #include "base/trace_event/common/trace_event_common.h" #include "include/v8-platform.h" #include "src/base/platform/platform.h" +#include "src/libplatform/tracing/recorder.h" namespace v8 { namespace platform { @@ -190,6 +191,27 @@ TraceWriter* TraceWriter::CreateJSONTraceWriter(std::ostream& stream, return new JSONTraceWriter(stream, tag); } +SystemInstrumentationTraceWriter::SystemInstrumentationTraceWriter() { + recorder_ = std::make_unique(); +} + +SystemInstrumentationTraceWriter::~SystemInstrumentationTraceWriter() { + recorder_.reset(nullptr); +} + +void SystemInstrumentationTraceWriter::AppendTraceEvent( + TraceObject* trace_event) { + if (recorder_->IsEnabled()) { + recorder_->AddEvent(trace_event); + } +} + +void SystemInstrumentationTraceWriter::Flush() {} + +TraceWriter* TraceWriter::CreateSystemInstrumentationTraceWriter() { + return new SystemInstrumentationTraceWriter(); +} + } // namespace tracing } // namespace platform } // namespace v8 diff --git a/src/libplatform/tracing/trace-writer.h b/src/libplatform/tracing/trace-writer.h index df48c5a377..1f727b815a 100644 --- a/src/libplatform/tracing/trace-writer.h +++ b/src/libplatform/tracing/trace-writer.h @@ -11,6 +11,8 @@ namespace v8 { namespace platform { namespace tracing { +class Recorder; + class JSONTraceWriter : public TraceWriter { public: explicit JSONTraceWriter(std::ostream& stream); @@ -27,6 +29,17 @@ class JSONTraceWriter : public TraceWriter { bool append_comma_ = false; }; +class SystemInstrumentationTraceWriter : public TraceWriter { + public: + SystemInstrumentationTraceWriter(); + ~SystemInstrumentationTraceWriter() override; + void AppendTraceEvent(TraceObject* trace_event) override; + void Flush() override; + + private: + std::unique_ptr recorder_; +}; + } // namespace tracing } // namespace platform } // namespace v8 diff --git a/tools/wpr.wprp b/tools/wpr.wprp new file mode 100644 index 0000000000..a6282490a8 --- /dev/null +++ b/tools/wpr.wprp @@ -0,0 +1,68 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +