From 1ab217ac409ad77ce0db3b053831f33a8f551fec Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Mon, 25 Jul 2022 17:42:49 +0000 Subject: [PATCH] Reland "[vm] Forward dynamic events names to os_signposts as arguments." Assume the embedder provides static strings for event labels. TEST=Instruments Bug: https://github.com/dart-lang/sdk/issues/49178 Change-Id: I40aa4996fed2b1230da64d182e6f172f60480fdf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251145 Reviewed-by: Siva Annamalai Commit-Queue: Ryan Macnak --- runtime/vm/dart_api_impl_test.cc | 3 +- runtime/vm/service.cc | 4 +-- runtime/vm/timeline.cc | 17 ++++++----- runtime/vm/timeline.h | 35 ++++++++++++--------- runtime/vm/timeline_macos.cc | 52 ++++++++++++++++++-------------- runtime/vm/timeline_test.cc | 16 +++++----- 6 files changed, 70 insertions(+), 57 deletions(-) diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 01a552f75b4..5230ed87aa9 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -9553,8 +9553,7 @@ TEST_CASE(DartAPI_TimelineCategories) { JSONArray jstream(&obj, "available"); Timeline::PrintFlagsToJSONArray(&jstream); const char* js_str = js.ToCString(); -#define TIMELINE_STREAM_CHECK(name, fuchsia_name) \ - EXPECT_SUBSTRING(#name, js_str); +#define TIMELINE_STREAM_CHECK(name, ...) EXPECT_SUBSTRING(#name, js_str); TIMELINE_STREAM_LIST(TIMELINE_STREAM_CHECK) #undef TIMELINE_STREAM_CHECK } diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc index c03a267ebf6..a455360c4aa 100644 --- a/runtime/vm/service.cc +++ b/runtime/vm/service.cc @@ -296,7 +296,7 @@ class EnumListParameter : public MethodParameter { #if defined(SUPPORT_TIMELINE) static const char* const timeline_streams_enum_names[] = { "all", -#define DEFINE_NAME(name, unused) #name, +#define DEFINE_NAME(name, ...) #name, TIMELINE_STREAM_LIST(DEFINE_NAME) #undef DEFINE_NAME NULL}; @@ -328,7 +328,7 @@ bool Service::EnableTimelineStreams(char* categories_list) { return false; } -#define SET_ENABLE_STREAM(name, unused) \ +#define SET_ENABLE_STREAM(name, ...) \ Timeline::SetStream##name##Enabled(HasStream(streams, #name)); TIMELINE_STREAM_LIST(SET_ENABLE_STREAM); #undef SET_ENABLE_STREAM diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc index 90b0a238287..f5140c602bb 100644 --- a/runtime/vm/timeline.cc +++ b/runtime/vm/timeline.cc @@ -202,7 +202,7 @@ void Timeline::Init() { ASSERT(recorder_ != NULL); enabled_streams_ = GetEnabledByDefaultTimelineStreams(); // Global overrides. -#define TIMELINE_STREAM_FLAG_DEFAULT(name, fuchsia_name) \ +#define TIMELINE_STREAM_FLAG_DEFAULT(name, ...) \ stream_##name##_.set_enabled(HasStream(enabled_streams_, #name)); TIMELINE_STREAM_LIST(TIMELINE_STREAM_FLAG_DEFAULT) #undef TIMELINE_STREAM_FLAG_DEFAULT @@ -218,7 +218,7 @@ void Timeline::Cleanup() { #endif // Disable global streams. -#define TIMELINE_STREAM_DISABLE(name, fuchsia_name) \ +#define TIMELINE_STREAM_DISABLE(name, ...) \ Timeline::stream_##name##_.set_enabled(false); TIMELINE_STREAM_LIST(TIMELINE_STREAM_DISABLE) #undef TIMELINE_STREAM_DISABLE @@ -265,7 +265,7 @@ void Timeline::ReclaimCachedBlocksFromThreadsUnsafe() { #ifndef PRODUCT void Timeline::PrintFlagsToJSONArray(JSONArray* arr) { -#define ADD_RECORDED_STREAM_NAME(name, fuchsia_name) \ +#define ADD_RECORDED_STREAM_NAME(name, ...) \ if (stream_##name##_.enabled()) { \ arr->AddValue(#name); \ } @@ -285,13 +285,13 @@ void Timeline::PrintFlagsToJSON(JSONStream* js) { } { JSONArray availableStreams(&obj, "availableStreams"); -#define ADD_STREAM_NAME(name, fuchsia_name) availableStreams.AddValue(#name); +#define ADD_STREAM_NAME(name, ...) availableStreams.AddValue(#name); TIMELINE_STREAM_LIST(ADD_STREAM_NAME); #undef ADD_STREAM_NAME } { JSONArray recordedStreams(&obj, "recordedStreams"); -#define ADD_RECORDED_STREAM_NAME(name, fuchsia_name) \ +#define ADD_RECORDED_STREAM_NAME(name, ...) \ if (stream_##name##_.enabled()) { \ recordedStreams.AddValue(#name); \ } @@ -402,8 +402,9 @@ TimelineEventRecorder* Timeline::recorder_ = NULL; MallocGrowableArray* Timeline::enabled_streams_ = NULL; bool Timeline::recorder_discards_clock_values_ = false; -#define TIMELINE_STREAM_DEFINE(name, fuchsia_name) \ - TimelineStream Timeline::stream_##name##_(#name, fuchsia_name, false); +#define TIMELINE_STREAM_DEFINE(name, fuchsia_name, static_labels) \ + TimelineStream Timeline::stream_##name##_(#name, fuchsia_name, \ + static_labels, false); TIMELINE_STREAM_LIST(TIMELINE_STREAM_DEFINE) #undef TIMELINE_STREAM_DEFINE @@ -774,6 +775,7 @@ int64_t TimelineEvent::ThreadCPUTimeDuration() const { TimelineStream::TimelineStream(const char* name, const char* fuchsia_name, + bool has_static_labels, bool enabled) : name_(name), fuchsia_name_(fuchsia_name), @@ -786,6 +788,7 @@ TimelineStream::TimelineStream(const char* name, #if defined(DART_HOST_OS_MACOS) if (__builtin_available(iOS 12.0, macOS 10.14, *)) { macos_log_ = os_log_create("Dart", name); + has_static_labels_ = has_static_labels; } #endif } diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h index 7b89e4b3bc8..2d06a1e31ff 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -57,23 +57,26 @@ class Zone; #define STARTUP_RECORDER_NAME "Startup" #define SYSTRACE_RECORDER_NAME "Systrace" -// (name, fuchsia_name). +// (name, fuchsia_name, has_static_labels). #define TIMELINE_STREAM_LIST(V) \ - V(API, "dart:api") \ - V(Compiler, "dart:compiler") \ - V(CompilerVerbose, "dart:compiler.verbose") \ - V(Dart, "dart:dart") \ - V(Debugger, "dart:debugger") \ - V(Embedder, "dart:embedder") \ - V(GC, "dart:gc") \ - V(Isolate, "dart:isolate") \ - V(VM, "dart:vm") + V(API, "dart:api", true) \ + V(Compiler, "dart:compiler", true) \ + V(CompilerVerbose, "dart:compiler.verbose", true) \ + V(Dart, "dart:dart", false) \ + V(Debugger, "dart:debugger", true) \ + V(Embedder, "dart:embedder", true) \ + V(GC, "dart:gc", true) \ + V(Isolate, "dart:isolate", true) \ + V(VM, "dart:vm", true) // A stream of timeline events. A stream has a name and can be enabled or // disabled (globally and per isolate). class TimelineStream { public: - TimelineStream(const char* name, const char* fuchsia_name, bool enabled); + TimelineStream(const char* name, + const char* fuchsia_name, + bool static_labels, + bool enabled); const char* name() const { return name_; } const char* fuchsia_name() const { return fuchsia_name_; } @@ -105,7 +108,8 @@ class TimelineStream { #if defined(DART_HOST_OS_FUCHSIA) trace_site_t* trace_site() { return &trace_site_; } #elif defined(DART_HOST_OS_MACOS) - os_log_t macos_log() { return macos_log_; } + os_log_t macos_log() const { return macos_log_; } + bool has_static_labels() const { return has_static_labels_; } #endif private: @@ -120,6 +124,7 @@ class TimelineStream { trace_site_t trace_site_ = {}; #elif defined(DART_HOST_OS_MACOS) os_log_t macos_log_ = {}; + bool has_static_labels_ = false; #endif }; @@ -196,12 +201,12 @@ class Timeline : public AllStatic { static void PrintFlagsToJSONArray(JSONArray* arr); #endif -#define TIMELINE_STREAM_ACCESSOR(name, fuchsia_name) \ +#define TIMELINE_STREAM_ACCESSOR(name, ...) \ static TimelineStream* Get##name##Stream() { return &stream_##name##_; } TIMELINE_STREAM_LIST(TIMELINE_STREAM_ACCESSOR) #undef TIMELINE_STREAM_ACCESSOR -#define TIMELINE_STREAM_FLAGS(name, fuchsia_name) \ +#define TIMELINE_STREAM_FLAGS(name, ...) \ static void SetStream##name##Enabled(bool enabled) { \ stream_##name##_.set_enabled(enabled); \ } @@ -216,7 +221,7 @@ class Timeline : public AllStatic { static MallocGrowableArray* enabled_streams_; static bool recorder_discards_clock_values_; -#define TIMELINE_STREAM_DECLARE(name, fuchsia_name) \ +#define TIMELINE_STREAM_DECLARE(name, ...) \ static TimelineStream stream_##name##_; TIMELINE_STREAM_LIST(TIMELINE_STREAM_DECLARE) #undef TIMELINE_STREAM_DECLARE diff --git a/runtime/vm/timeline_macos.cc b/runtime/vm/timeline_macos.cc index d1876177834..55e0b2435ea 100644 --- a/runtime/vm/timeline_macos.cc +++ b/runtime/vm/timeline_macos.cc @@ -30,39 +30,45 @@ void TimelineEventMacosRecorder::OnEvent(TimelineEvent* event) { } const char* label = event->label(); + bool is_static_label = event->stream_->has_static_labels(); uint8_t _Alignas(16) buffer[64]; buffer[0] = 0; switch (event->event_type()) { - case TimelineEvent::kInstant: { - _os_signpost_emit_with_name_impl(&__dso_handle, log, OS_SIGNPOST_EVENT, - OS_SIGNPOST_ID_EXCLUSIVE, label, "", - buffer, sizeof(buffer)); + case TimelineEvent::kInstant: + if (is_static_label) { + _os_signpost_emit_with_name_impl(&__dso_handle, log, OS_SIGNPOST_EVENT, + OS_SIGNPOST_ID_EXCLUSIVE, label, "", + buffer, sizeof(buffer)); + } else { + os_signpost_event_emit(log, OS_SIGNPOST_ID_EXCLUSIVE, "Event", "%s", + label); + } break; - } case TimelineEvent::kBegin: - case TimelineEvent::kAsyncBegin: { - _os_signpost_emit_with_name_impl(&__dso_handle, log, - OS_SIGNPOST_INTERVAL_BEGIN, event->Id(), - label, "", buffer, sizeof(buffer)); + case TimelineEvent::kAsyncBegin: + if (is_static_label) { + _os_signpost_emit_with_name_impl( + &__dso_handle, log, OS_SIGNPOST_INTERVAL_BEGIN, event->Id(), label, + "", buffer, sizeof(buffer)); + } else { + os_signpost_interval_begin(log, event->Id(), "Event", "%s", label); + } break; - } case TimelineEvent::kEnd: - case TimelineEvent::kAsyncEnd: { - _os_signpost_emit_with_name_impl(&__dso_handle, log, - OS_SIGNPOST_INTERVAL_END, event->Id(), - label, "", buffer, sizeof(buffer)); + case TimelineEvent::kAsyncEnd: + if (is_static_label) { + _os_signpost_emit_with_name_impl(&__dso_handle, log, + OS_SIGNPOST_INTERVAL_END, event->Id(), + label, "", buffer, sizeof(buffer)); + } else { + os_signpost_interval_end(log, event->Id(), "Event"); + } break; - } - case TimelineEvent::kCounter: { - const char* fmt = "%s"; - Utils::SNPrint(reinterpret_cast(buffer), sizeof(buffer), fmt, - event->arguments()[0].value); - _os_signpost_emit_with_name_impl(&__dso_handle, log, OS_SIGNPOST_EVENT, - OS_SIGNPOST_ID_EXCLUSIVE, label, fmt, - buffer, sizeof(buffer)); + case TimelineEvent::kCounter: + os_signpost_event_emit(log, OS_SIGNPOST_ID_EXCLUSIVE, "Counter", "%s=%s", + label, event->arguments()[0].value); break; - } default: break; } diff --git a/runtime/vm/timeline_test.cc b/runtime/vm/timeline_test.cc index 3c5ec0414e4..09d226bc5a7 100644 --- a/runtime/vm/timeline_test.cc +++ b/runtime/vm/timeline_test.cc @@ -105,7 +105,7 @@ class TimelineTestHelper : public AllStatic { TEST_CASE(TimelineEventIsValid) { // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); TimelineEvent event; TimelineTestHelper::SetStream(&event, &stream); @@ -124,7 +124,7 @@ TEST_CASE(TimelineEventIsValid) { TEST_CASE(TimelineEventDuration) { // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); // Create a test event. TimelineEvent event; @@ -139,7 +139,7 @@ TEST_CASE(TimelineEventDuration) { TEST_CASE(TimelineEventDurationPrintJSON) { // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); // Create a test event. TimelineEvent event; @@ -169,7 +169,7 @@ TEST_CASE(TimelineEventPrintSystrace) { char buffer[kBufferLength]; // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); // Create a test event. TimelineEvent event; @@ -213,7 +213,7 @@ TEST_CASE(TimelineEventPrintSystrace) { TEST_CASE(TimelineEventArguments) { // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); // Create a test event. TimelineEvent event; @@ -233,7 +233,7 @@ TEST_CASE(TimelineEventArguments) { TEST_CASE(TimelineEventArgumentsPrintJSON) { // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); // Create a test event. TimelineEvent event; @@ -296,7 +296,7 @@ TEST_CASE(TimelineEventCallbackRecorderBasic) { } // Create a test stream. - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); TimelineEvent* event = NULL; @@ -341,7 +341,7 @@ TEST_CASE(TimelineEventCallbackRecorderBasic) { } TEST_CASE(TimelineRingRecorderJSONOrder) { - TimelineStream stream("testStream", "testStream", true); + TimelineStream stream("testStream", "testStream", false, true); TimelineEventRingRecorder* recorder = new TimelineEventRingRecorder(TimelineEventBlock::kBlockSize * 2);