From 193c4ccbb9cbfdc157fb3efce1b4337d0cc46cb7 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Fri, 26 May 2023 13:08:27 +0000 Subject: [PATCH] [VM/Timeline] Store an array of flow IDs in each dart::TimelineEvent instead of just one ID Sometimes the Flutter Engine is associating events with multiple flows, this change is needed to allow events to be associated with multiple flows in a Perfetto-format trace. TEST=Checked the flow events reported through dart:developer still looked correct when retrieved in Perfetto traces. Change-Id: I2901ffde5e8b984abb1e924e014722bb0568f6d3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305801 Reviewed-by: Ben Konyi Commit-Queue: Derek Xu --- runtime/lib/timeline.cc | 10 ++++++++- runtime/vm/dart_api_impl.cc | 3 +-- runtime/vm/timeline.cc | 42 +++++++++++++++++++++---------------- runtime/vm/timeline.h | 21 ++++++++++--------- runtime/vm/timeline_test.cc | 2 +- 5 files changed, 46 insertions(+), 32 deletions(-) diff --git a/runtime/lib/timeline.cc b/runtime/lib/timeline.cc index 64e63d68a0a..4592e50aba0 100644 --- a/runtime/lib/timeline.cc +++ b/runtime/lib/timeline.cc @@ -55,8 +55,16 @@ DEFINE_NATIVE_ENTRY(Timeline_reportTaskEvent, 0, 5) { return Object::null(); } + std::unique_ptr flow_ids; + if (flow_id.AsInt64Value() != TimelineEvent::kNoFlowId) { + int64_t* flow_ids_internal = new int64_t[1]; + flow_ids_internal[0] = flow_id.AsInt64Value(); + flow_ids = std::unique_ptr(flow_ids_internal); + } + intptr_t flow_id_count = + flow_id.AsInt64Value() == TimelineEvent::kNoFlowId ? 0 : 1; DartTimelineEventHelpers::ReportTaskEvent( - event, id.AsInt64Value(), flow_id.AsInt64Value(), type.Value(), + event, id.AsInt64Value(), flow_id_count, flow_ids, type.Value(), name.ToMallocCString(), args.ToMallocCString()); #endif // SUPPORT_TIMELINE return Object::null(); diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index 55b98e935ca..7c42f262f0a 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -6443,8 +6443,7 @@ DART_EXPORT void Dart_TimelineEvent(const char* label, // TODO(derekx): Dart_TimelineEvent() needs to be updated so that arrows // corresponding to flow events reported by embedders get included in // Perfetto traces. - event->Begin(label, timestamp1_or_async_id, - /*flow_id=*/TimelineEvent::kNoFlowId, timestamp0); + event->Begin(label, timestamp1_or_async_id, timestamp0); break; case Dart_Timeline_Event_End: event->End(label, timestamp1_or_async_id, timestamp0); diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc index 94da8fc2c9a..1f63befd7d4 100644 --- a/runtime/vm/timeline.cc +++ b/runtime/vm/timeline.cc @@ -505,7 +505,8 @@ TIMELINE_STREAM_LIST(TIMELINE_STREAM_DEFINE) TimelineEvent::TimelineEvent() : timestamp0_(0), timestamp1_(0), - flow_id_(TimelineEvent::kNoFlowId), + flow_id_count_(0), + flow_ids_(), state_(0), label_(nullptr), stream_(nullptr), @@ -585,14 +586,12 @@ void TimelineEvent::Duration(const char* label, void TimelineEvent::Begin(const char* label, int64_t id, - int64_t flow_id, int64_t micros) { Init(kBegin, label); set_timestamp0(micros); // Overload timestamp1_ with the task id. This is required for the MacOS // recorder to work. set_timestamp1(id); - set_flow_id(flow_id); } void TimelineEvent::End(const char* label, int64_t id, int64_t micros) { @@ -670,7 +669,8 @@ void TimelineEvent::Init(EventType event_type, const char* label) { state_ = 0; timestamp0_ = 0; timestamp1_ = 0; - flow_id_ = TimelineEvent::kNoFlowId; + flow_id_count_ = 0; + flow_ids_.reset(); OSThread* os_thread = OSThread::Current(); ASSERT(os_thread != nullptr); thread_ = os_thread->trace_id(); @@ -841,12 +841,12 @@ inline void AddBeginEventFields( AddBeginAndInstantEventCommonFields(track_event, event); track_event->set_type( perfetto::protos::pbzero::TrackEvent::Type::TYPE_SLICE_BEGIN); - if (event.HasFlowId()) { + for (intptr_t i = 0; i < event.flow_id_count(); ++i) { // TODO(derekx): |TrackEvent|s have a |terminating_flow_ids| field that we // aren't able to populate right now because we aren't keeping track of // terminating flow IDs in |TimelineEvent|. I'm not even sure if using that // field will provide any benefit though. - track_event->add_flow_ids(event.flow_id()); + track_event->add_flow_ids(event.FlowIds()[i]); } } @@ -985,10 +985,6 @@ int64_t TimelineEvent::TimeDuration() const { return timestamp1_ - timestamp0_; } -bool TimelineEvent::HasFlowId() const { - return flow_id_ != TimelineEvent::kNoFlowId; -} - bool TimelineEvent::HasIsolateId() const { return isolate_id_ != ILLEGAL_ISOLATE_ID; } @@ -1240,7 +1236,7 @@ void TimelineBeginEndScope::EmitBegin() { } ASSERT(event != nullptr); // Emit a begin event. - event->Begin(label(), id(), /*flow_id=*/TimelineEvent::kNoFlowId); + event->Begin(label(), id()); event->Complete(); } @@ -2416,12 +2412,14 @@ void TimelineEventBlock::Finish() { #endif } -void DartTimelineEventHelpers::ReportTaskEvent(TimelineEvent* event, - int64_t id, - int64_t flow_id, - intptr_t type, - char* name, - char* args) { +void DartTimelineEventHelpers::ReportTaskEvent( + TimelineEvent* event, + int64_t id, + intptr_t flow_id_count, + std::unique_ptr& flow_ids, + intptr_t type, + char* name, + char* args) { const int64_t start = OS::GetCurrentMonotonicMicrosForTimeline(); switch (static_cast(type)) { case TimelineEvent::kAsyncInstant: @@ -2434,7 +2432,7 @@ void DartTimelineEventHelpers::ReportTaskEvent(TimelineEvent* event, event->AsyncEnd(name, id, start); break; case TimelineEvent::kBegin: - event->Begin(name, id, flow_id, start); + event->Begin(name, id, start); break; case TimelineEvent::kEnd: event->End(name, id, start); @@ -2454,6 +2452,14 @@ void DartTimelineEventHelpers::ReportTaskEvent(TimelineEvent* event, default: UNREACHABLE(); } + if (flow_id_count > 0) { + ASSERT(type == Dart_Timeline_Event_Begin || + type == Dart_Timeline_Event_Instant || + type == Dart_Timeline_Event_Async_Begin || + type == Dart_Timeline_Event_Async_Instant); + + event->SetFlowIds(flow_id_count, flow_ids); + } event->set_owns_label(true); event->CompleteWithPreSerializedArgs(args); } diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h index 7bb39f54cce..665059911fb 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -373,7 +373,6 @@ class TimelineEvent { void Begin(const char* label, int64_t id, - int64_t flow_id, int64_t micros = OS::GetCurrentMonotonicMicrosForTimeline()); void End(const char* label, @@ -441,8 +440,13 @@ class TimelineEvent { int64_t timestamp0() const { return timestamp0_; } int64_t timestamp1() const { return timestamp1_; } - bool HasFlowId() const; - int64_t flow_id() const { return flow_id_; } + void SetFlowIds(intptr_t flow_id_count, + std::unique_ptr& flow_ids) { + flow_id_count_ = flow_id_count; + flow_ids_.swap(flow_ids); + } + intptr_t flow_id_count() const { return flow_id_count_; } + const int64_t* FlowIds() const { return flow_ids_.get(); } bool HasIsolateId() const; bool HasIsolateGroupId() const; @@ -563,11 +567,6 @@ class TimelineEvent { timestamp1_ = value; } - void set_flow_id(int64_t flow_id) { - ASSERT(flow_id_ == TimelineEvent::kNoFlowId); - flow_id_ = flow_id; - } - void set_pre_serialized_args(bool pre_serialized_args) { state_ = PreSerializedArgsBit::update(pre_serialized_args, state_); } @@ -588,12 +587,13 @@ class TimelineEvent { int64_t timestamp0_; int64_t timestamp1_; + intptr_t flow_id_count_; // This field is only used by the Perfetto recorders, because Perfetto's trace // format handles flow events by processing flow IDs attached to // |TimelineEvent::kBegin| events. Other recorders handle flow events by // processing events of type TimelineEvent::kFlowBegin|, // |TimelineEvent::kFlowStep|, and |TimelineEvent::kFlowEnd|. - int64_t flow_id_; + std::unique_ptr flow_ids_; TimelineEventArguments arguments_; uword state_; const char* label_; @@ -1293,7 +1293,8 @@ class DartTimelineEventHelpers : public AllStatic { public: static void ReportTaskEvent(TimelineEvent* event, int64_t id, - int64_t flow_id, + intptr_t flow_id_count, + std::unique_ptr& flow_ids, intptr_t type, char* name, char* args); diff --git a/runtime/vm/timeline_test.cc b/runtime/vm/timeline_test.cc index 828775c5b66..10500735375 100644 --- a/runtime/vm/timeline_test.cc +++ b/runtime/vm/timeline_test.cc @@ -85,7 +85,7 @@ class TimelineTestHelper : public AllStatic { ASSERT(start >= 0); TimelineEvent* event = recorder->StartEvent(); ASSERT(event != nullptr); - event->Begin(label, /*id=*/-1, /*flow_id=*/TimelineEvent::kNoFlowId, start); + event->Begin(label, /*id=*/-1, start); event->Complete(); }