Reland "[vm] Forward dynamic events names to os_signposts as arguments."

rmacnak-google · Commit Bot · commit 1ab217ac409a · 2022-07-25T17:42:49.000Z
Assume the embedder provides static strings for event labels. TEST=Instruments Bug: #49178 Change-Id: I40aa4996fed2b1230da64d182e6f172f60480fdf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251145 Reviewed-by: Siva Annamalai <asiva@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
diff --git 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
@@ -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
@@ -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<char*>* 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
@@ -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<char*>* 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
@@ -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<char*>(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
@@ -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);
