From f7990bc0da61b252d80f953aecfac9ef5fd682aa Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 1 Jul 2022 16:07:50 +0000 Subject: [PATCH] Revert "[vm] Forward dynamic events names to os_signposts as arguments." This reverts commit 90da09e2db96ba3e7a2c938407afb3ee514450bf. Reason for revert: Blocking this revert: https://dart-review.googlesource.com/c/sdk/+/250403 Original change's description: > [vm] Forward dynamic events names to os_signposts as arguments. > > TEST=Instruments > Bug: https://github.com/dart-lang/sdk/issues/49178 > Change-Id: Ice4a83f03dccb6c7dbd54c229f37d4ea7d0d8051 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250111 > Reviewed-by: Siva Annamalai > Commit-Queue: Ryan Macnak TBR=rmacnak@google.com,asiva@google.com Change-Id: I326fb50ead5c298e62666371adadb2b9f40d69c7 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: https://github.com/dart-lang/sdk/issues/49178 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250407 Reviewed-by: Siva Annamalai Reviewed-by: Liam Appelbe Reviewed-by: Alexander Aprelev Commit-Queue: Liam Appelbe --- 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, 57 insertions(+), 70 deletions(-) diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 98a84ecae1f..0e7442a1138 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -9500,7 +9500,8 @@ TEST_CASE(DartAPI_TimelineCategories) { JSONArray jstream(&obj, "available"); Timeline::PrintFlagsToJSONArray(&jstream); const char* js_str = js.ToCString(); -#define TIMELINE_STREAM_CHECK(name, ...) EXPECT_SUBSTRING(#name, js_str); +#define TIMELINE_STREAM_CHECK(name, fuchsia_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 af7a225e6db..85cf4038ac2 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, ...) #name, +#define DEFINE_NAME(name, unused) #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, ...) \ +#define SET_ENABLE_STREAM(name, unused) \ 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 f5140c602bb..90b0a238287 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, ...) \ +#define TIMELINE_STREAM_FLAG_DEFAULT(name, fuchsia_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, ...) \ +#define TIMELINE_STREAM_DISABLE(name, fuchsia_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, ...) \ +#define ADD_RECORDED_STREAM_NAME(name, fuchsia_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, ...) availableStreams.AddValue(#name); +#define ADD_STREAM_NAME(name, fuchsia_name) availableStreams.AddValue(#name); TIMELINE_STREAM_LIST(ADD_STREAM_NAME); #undef ADD_STREAM_NAME } { JSONArray recordedStreams(&obj, "recordedStreams"); -#define ADD_RECORDED_STREAM_NAME(name, ...) \ +#define ADD_RECORDED_STREAM_NAME(name, fuchsia_name) \ if (stream_##name##_.enabled()) { \ recordedStreams.AddValue(#name); \ } @@ -402,9 +402,8 @@ TimelineEventRecorder* Timeline::recorder_ = NULL; MallocGrowableArray* Timeline::enabled_streams_ = NULL; bool Timeline::recorder_discards_clock_values_ = false; -#define TIMELINE_STREAM_DEFINE(name, fuchsia_name, static_labels) \ - TimelineStream Timeline::stream_##name##_(#name, fuchsia_name, \ - static_labels, false); +#define TIMELINE_STREAM_DEFINE(name, fuchsia_name) \ + TimelineStream Timeline::stream_##name##_(#name, fuchsia_name, false); TIMELINE_STREAM_LIST(TIMELINE_STREAM_DEFINE) #undef TIMELINE_STREAM_DEFINE @@ -775,7 +774,6 @@ 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), @@ -788,7 +786,6 @@ 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 8071185307e..7b89e4b3bc8 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -57,26 +57,23 @@ class Zone; #define STARTUP_RECORDER_NAME "Startup" #define SYSTRACE_RECORDER_NAME "Systrace" -// (name, fuchsia_name, has_static_labels). +// (name, fuchsia_name). #define TIMELINE_STREAM_LIST(V) \ - 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", false) \ - V(GC, "dart:gc", true) \ - V(Isolate, "dart:isolate", true) \ - V(VM, "dart:vm", true) + 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") // 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 static_labels, - bool enabled); + TimelineStream(const char* name, const char* fuchsia_name, bool enabled); const char* name() const { return name_; } const char* fuchsia_name() const { return fuchsia_name_; } @@ -108,8 +105,7 @@ 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() const { return macos_log_; } - bool has_static_labels() const { return has_static_labels_; } + os_log_t macos_log() { return macos_log_; } #endif private: @@ -124,7 +120,6 @@ class TimelineStream { trace_site_t trace_site_ = {}; #elif defined(DART_HOST_OS_MACOS) os_log_t macos_log_ = {}; - bool has_static_labels_ = false; #endif }; @@ -201,12 +196,12 @@ class Timeline : public AllStatic { static void PrintFlagsToJSONArray(JSONArray* arr); #endif -#define TIMELINE_STREAM_ACCESSOR(name, ...) \ +#define TIMELINE_STREAM_ACCESSOR(name, fuchsia_name) \ static TimelineStream* Get##name##Stream() { return &stream_##name##_; } TIMELINE_STREAM_LIST(TIMELINE_STREAM_ACCESSOR) #undef TIMELINE_STREAM_ACCESSOR -#define TIMELINE_STREAM_FLAGS(name, ...) \ +#define TIMELINE_STREAM_FLAGS(name, fuchsia_name) \ static void SetStream##name##Enabled(bool enabled) { \ stream_##name##_.set_enabled(enabled); \ } @@ -221,7 +216,7 @@ class Timeline : public AllStatic { static MallocGrowableArray* enabled_streams_; static bool recorder_discards_clock_values_; -#define TIMELINE_STREAM_DECLARE(name, ...) \ +#define TIMELINE_STREAM_DECLARE(name, fuchsia_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 55e0b2435ea..d1876177834 100644 --- a/runtime/vm/timeline_macos.cc +++ b/runtime/vm/timeline_macos.cc @@ -30,45 +30,39 @@ 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: - 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); - } + case TimelineEvent::kInstant: { + _os_signpost_emit_with_name_impl(&__dso_handle, log, OS_SIGNPOST_EVENT, + OS_SIGNPOST_ID_EXCLUSIVE, label, "", + buffer, sizeof(buffer)); break; + } case TimelineEvent::kBegin: - 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); - } + case TimelineEvent::kAsyncBegin: { + _os_signpost_emit_with_name_impl(&__dso_handle, log, + OS_SIGNPOST_INTERVAL_BEGIN, event->Id(), + label, "", buffer, sizeof(buffer)); break; + } case TimelineEvent::kEnd: - 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"); - } + case TimelineEvent::kAsyncEnd: { + _os_signpost_emit_with_name_impl(&__dso_handle, log, + OS_SIGNPOST_INTERVAL_END, event->Id(), + label, "", buffer, sizeof(buffer)); break; - case TimelineEvent::kCounter: - os_signpost_event_emit(log, OS_SIGNPOST_ID_EXCLUSIVE, "Counter", "%s=%s", - label, event->arguments()[0].value); + } + 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)); break; + } default: break; } diff --git a/runtime/vm/timeline_test.cc b/runtime/vm/timeline_test.cc index 09d226bc5a7..3c5ec0414e4 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", false, true); + TimelineStream stream("testStream", "testStream", 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", false, true); + TimelineStream stream("testStream", "testStream", 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", false, true); + TimelineStream stream("testStream", "testStream", 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", false, true); + TimelineStream stream("testStream", "testStream", 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", false, true); + TimelineStream stream("testStream", "testStream", 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", false, true); + TimelineStream stream("testStream", "testStream", true); // Create a test event. TimelineEvent event; @@ -296,7 +296,7 @@ TEST_CASE(TimelineEventCallbackRecorderBasic) { } // Create a test stream. - TimelineStream stream("testStream", "testStream", false, true); + TimelineStream stream("testStream", "testStream", true); TimelineEvent* event = NULL; @@ -341,7 +341,7 @@ TEST_CASE(TimelineEventCallbackRecorderBasic) { } TEST_CASE(TimelineRingRecorderJSONOrder) { - TimelineStream stream("testStream", "testStream", false, true); + TimelineStream stream("testStream", "testStream", true); TimelineEventRingRecorder* recorder = new TimelineEventRingRecorder(TimelineEventBlock::kBlockSize * 2);