From 3787601dd0b9a7abe1d1052d2ca28f3cfb0f68da Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Fri, 3 Feb 2023 21:40:58 +0000 Subject: [PATCH] [VM/Timeline] Improve handling of track metadata The track information that we send over the service is currently incomplete. Since we only iterate over the threads that exist at the time when getVMTimeline is called, we're missing information about any threads that were joined before that. This CL fixes this issue. TEST=CI, ASAN, and I manually checked that more track names are populated when the changes in this CL are applied Change-Id: Ic4edc9910884c3f4743dc0ca65aa7d2a695ff09d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280058 Reviewed-by: Ben Konyi Commit-Queue: Derek Xu --- runtime/vm/timeline.cc | 102 ++++++++++++++++++++++++++++++----------- runtime/vm/timeline.h | 42 ++++++++++++++--- 2 files changed, 111 insertions(+), 33 deletions(-) diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc index d8087025e33..24195c2fff1 100644 --- a/runtime/vm/timeline.cc +++ b/runtime/vm/timeline.cc @@ -11,8 +11,10 @@ #include #include +#include #include "platform/atomic.h" +#include "platform/hashmap.h" #include "vm/isolate.h" #include "vm/json_stream.h" #include "vm/lockers.h" @@ -629,6 +631,16 @@ void TimelineEvent::Init(EventType event_type, const char* label) { OSThread* os_thread = OSThread::Current(); ASSERT(os_thread != NULL); thread_ = os_thread->trace_id(); + if (FLAG_timeline_recorder != nullptr && + // There is no way to retrieve track metadata when a callback or systrace + // recorder is in use, so we don't need to call + // |AddTrackMetadataBasedOnThread()| in these cases. + strcmp("callback", FLAG_timeline_recorder) != 0 && + strcmp("systrace", FLAG_timeline_recorder) != 0) { + RecorderLockScope rl; + TimelineEventRecorder* recorder = Timeline::recorder(); + recorder->AddTrackMetadataBasedOnThread(*os_thread); + } auto thread = Thread::Current(); auto isolate = thread != nullptr ? thread->isolate() : nullptr; auto isolate_group = thread != nullptr ? thread->isolate_group() : nullptr; @@ -841,6 +853,27 @@ const char* TimelineEvent::GetFormattedIsolateGroupId() const { return formatted_isolate_group_id; } +TimelineTrackMetadata::TimelineTrackMetadata( + intptr_t pid, + intptr_t tid, + Utils::CStringUniquePtr&& track_name) + : pid_(pid), tid_(tid), track_name_(std::move(track_name)) {} + +#if !defined(PRODUCT) +void TimelineTrackMetadata::PrintJSON(const JSONArray& jsarr_events) const { + JSONObject jsobj(&jsarr_events); + jsobj.AddProperty("name", "thread_name"); + jsobj.AddProperty("ph", "M"); + jsobj.AddProperty("pid", pid()); + jsobj.AddProperty("tid", tid()); + { + JSONObject jsobj_args(&jsobj, "args"); + jsobj_args.AddPropertyF("name", "%s (%" Pd ")", track_name(), tid()); + jsobj_args.AddProperty("mode", "basic"); + } +} +#endif // !defined(PRODUCT) + TimelineStream::TimelineStream(const char* name, const char* fuchsia_name, bool has_static_labels, @@ -1034,30 +1067,28 @@ IsolateTimelineEventFilter::IsolateTimelineEventFilter( isolate_id_(isolate_id) {} TimelineEventRecorder::TimelineEventRecorder() - : time_low_micros_(0), time_high_micros_(0) {} + : time_low_micros_(0), + time_high_micros_(0), + track_uuid_to_track_metadata_( + &SimpleHashMap::SamePointerValue, + TimelineEventRecorder::kTrackUuidToTrackMetadataInitialCapacity) {} + +TimelineEventRecorder::~TimelineEventRecorder() { + for (SimpleHashMap::Entry* entry = track_uuid_to_track_metadata_.Start(); + entry != nullptr; entry = track_uuid_to_track_metadata_.Next(entry)) { + TimelineTrackMetadata* value = + static_cast(entry->value); + delete value; + } +} #ifndef PRODUCT -void TimelineEventRecorder::PrintJSONMeta(JSONArray* events) const { - OSThreadIterator it; - while (it.HasNext()) { - OSThread* thread = it.Next(); - const char* thread_name = thread->name(); - if (thread_name == NULL) { - // Only emit a thread name if one was set. - continue; - } - JSONObject obj(events); - int64_t pid = OS::ProcessId(); - int64_t tid = OSThread::ThreadIdToIntPtr(thread->trace_id()); - obj.AddProperty("name", "thread_name"); - obj.AddProperty("ph", "M"); - obj.AddProperty64("pid", pid); - obj.AddProperty64("tid", tid); - { - JSONObject args(&obj, "args"); - args.AddPropertyF("name", "%s (%" Pd64 ")", thread_name, tid); - args.AddProperty("mode", "basic"); - } +void TimelineEventRecorder::PrintJSONMeta(const JSONArray& jsarr_events) const { + for (SimpleHashMap::Entry* entry = track_uuid_to_track_metadata_.Start(); + entry != nullptr; entry = track_uuid_to_track_metadata_.Next(entry)) { + TimelineTrackMetadata* value = + static_cast(entry->value); + value->PrintJSON(jsarr_events); } } #endif @@ -1209,6 +1240,25 @@ TimelineEventBlock* TimelineEventRecorder::GetNewBlock() { return GetNewBlockLocked(); } +void TimelineEventRecorder::AddTrackMetadataBasedOnThread( + const OSThread& thread) { + intptr_t pid = OS::ProcessId(); + intptr_t tid = OSThread::ThreadIdToIntPtr(thread.trace_id()); + const char* thread_name = thread.name(); + + void* key = reinterpret_cast(tid); + const intptr_t hash = Utils::WordHash(tid); + SimpleHashMap::Entry* entry = + track_uuid_to_track_metadata_.Lookup(key, hash, true); + if (entry->value == nullptr) { + TimelineTrackMetadata* metadata = new TimelineTrackMetadata( + pid, tid, + Utils::CreateCStringUniquePtr( + Utils::StrDup(thread_name == nullptr ? "" : thread_name))); + entry->value = metadata; + } +} + TimelineEventFixedBufferRecorder::TimelineEventFixedBufferRecorder( intptr_t capacity) : memory_(NULL), @@ -1282,7 +1332,7 @@ void TimelineEventFixedBufferRecorder::PrintJSON(JSONStream* js, topLevel.AddProperty("type", "Timeline"); { JSONArray events(&topLevel, "traceEvents"); - PrintJSONMeta(&events); + PrintJSONMeta(events); PrintJSONEvents(&events, filter); } topLevel.AddPropertyTimeMicros("timeOriginMicros", TimeOriginMicros()); @@ -1293,7 +1343,7 @@ void TimelineEventFixedBufferRecorder::PrintTraceEvent( JSONStream* js, TimelineEventFilter* filter) { JSONArray events(js); - PrintJSONMeta(&events); + PrintJSONMeta(events); PrintJSONEvents(&events, filter); } #endif @@ -1638,7 +1688,7 @@ void TimelineEventEndlessRecorder::PrintJSON(JSONStream* js, topLevel.AddProperty("type", "Timeline"); { JSONArray events(&topLevel, "traceEvents"); - PrintJSONMeta(&events); + PrintJSONMeta(events); PrintJSONEvents(&events, filter); } topLevel.AddPropertyTimeMicros("timeOriginMicros", TimeOriginMicros()); @@ -1649,7 +1699,7 @@ void TimelineEventEndlessRecorder::PrintTraceEvent( JSONStream* js, TimelineEventFilter* filter) { JSONArray events(js); - PrintJSONMeta(&events); + PrintJSONMeta(events); PrintJSONEvents(&events, filter); } #endif diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h index 355fd1984be..471c4f9f617 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -8,6 +8,7 @@ #include "include/dart_tools_api.h" #include "platform/atomic.h" +#include "platform/hashmap.h" #include "vm/allocation.h" #include "vm/bitfield.h" #include "vm/globals.h" @@ -586,6 +587,31 @@ class TimelineEvent { DISALLOW_COPY_AND_ASSIGN(TimelineEvent); }; +class TimelineTrackMetadata { + public: + TimelineTrackMetadata(intptr_t pid, + intptr_t tid, + Utils::CStringUniquePtr&& track_name); + intptr_t pid() const { return pid_; } + intptr_t tid() const { return tid_; } + const char* track_name() const { return track_name_.get(); } +#if !defined(PRODUCT) + /* + * Prints a Chrome-format event representing the metadata stored by this + * object into |jsarr_events|. + */ + void PrintJSON(const JSONArray& jsarr_events) const; +#endif // !defined(PRODUCT) + + private: + // The ID of the process that this track is associated with. + intptr_t pid_; + // The trace ID of the thread that this track is associated with. + intptr_t tid_; + // The name of this track. + Utils::CStringUniquePtr track_name_; +}; + #ifdef SUPPORT_TIMELINE #define TIMELINE_DURATION(thread, stream, name) \ TimelineBeginEndScope tbes(thread, Timeline::Get##stream##Stream(), name); @@ -808,9 +834,7 @@ class IsolateTimelineEventFilter : public TimelineEventFilter { class TimelineEventRecorder : public MallocAllocated { public: TimelineEventRecorder(); - virtual ~TimelineEventRecorder() {} - - TimelineEventBlock* GetNewBlock(); + virtual ~TimelineEventRecorder(); // Interface method(s) which must be implemented. #ifndef PRODUCT @@ -818,10 +842,12 @@ class TimelineEventRecorder : public MallocAllocated { virtual void PrintTraceEvent(JSONStream* js, TimelineEventFilter* filter) = 0; #endif virtual const char* name() const = 0; - - void FinishBlock(TimelineEventBlock* block); - virtual intptr_t Size() = 0; + TimelineEventBlock* GetNewBlock(); + void FinishBlock(TimelineEventBlock* block); + // This function must be called at least once for each thread that corresponds + // to a track in the trace. + void AddTrackMetadataBasedOnThread(const OSThread& thread); protected: #ifndef PRODUCT @@ -837,7 +863,7 @@ class TimelineEventRecorder : public MallocAllocated { // Utility method(s). #ifndef PRODUCT - void PrintJSONMeta(JSONArray* array) const; + void PrintJSONMeta(const JSONArray& jsarr_events) const; #endif TimelineEvent* ThreadBlockStartEvent(); void ThreadBlockCompleteEvent(TimelineEvent* event); @@ -857,6 +883,8 @@ class TimelineEventRecorder : public MallocAllocated { friend class Timeline; private: + static const intptr_t kTrackUuidToTrackMetadataInitialCapacity = 1 << 4; + SimpleHashMap track_uuid_to_track_metadata_; DISALLOW_COPY_AND_ASSIGN(TimelineEventRecorder); };