Don't use SuspendAllScope for starting and dumping low overhead trace

We do a significant amount of work when starting / dumping low overhead
traces. It is not good to suspend all threads for such a long time. This
CL changes the implementation to use checkpoints and wait for all the
threads to run the checkpoint before proceeding.

Bug: 352518093
Test: art/test.py
Change-Id: I7464e89c1d9fb12c1e6aad6cc3ca2d3a35a27da7

2 files changed, 141 insertions, 82 deletions

diff --git a/runtime/trace_profile.cc b/runtime/trace_profile.cc
index 6aa09303d4..6c1476b5f9 100644
--- a/runtime/trace_profile.cc
+++ b/runtime/trace_profile.cc
@@ -67,6 +67,7 @@ int TraceProfiler::num_trace_stop_tasks_ = 0;
 TraceData* TraceProfiler::trace_data_ = nullptr;
 
 void TraceData::AddTracedThread(Thread* thread) {
+  MutexLock mu(Thread::Current(), trace_data_lock_);
   size_t thread_id = thread->GetTid();
   if (traced_threads_.find(thread_id) != traced_threads_.end()) {
     return;
@@ -106,9 +107,7 @@ LowOverheadTraceType TraceProfiler::GetTraceType() {
 
 namespace {
 void RecordMethodsOnThreadStack(Thread* thread, TraceData* trace_data)
-    REQUIRES(Locks::mutator_lock_) {
-  Locks::mutator_lock_->AssertExclusiveHeld(Thread::Current());
-
+    REQUIRES_SHARED(Locks::mutator_lock_) {
   struct MethodEntryStackVisitor final : public StackVisitor {
     MethodEntryStackVisitor(Thread* thread_in, Context* context)
         : StackVisitor(thread_in, context, StackVisitor::StackWalkKind::kSkipInlinedFrames) {}
@@ -145,11 +144,12 @@ void RecordMethodsOnThreadStack(Thread* thread, TraceData* trace_data)
   // isn't accurate.
   uintptr_t* method_trace_buffer = thread->GetMethodTraceBuffer();
   uint64_t init_time = TimestampCounter::GetMicroTime(TimestampCounter::GetTimestamp());
+  std::unordered_set<ArtMethod*> methods;
   std::ostringstream os;
   os << "Thread:" << thread->GetTid() << "\n";
   for (auto smi = visitor.stack_methods_.rbegin(); smi != visitor.stack_methods_.rend(); smi++) {
     os << "S->" << *smi << " " << init_time << "\n";
-    trace_data->AddTracedMethod(*smi);
+    methods.insert(*smi);
   }
 
   // Record a placeholder method exit event into the buffer so we record method exits for the
@@ -158,6 +158,27 @@ void RecordMethodsOnThreadStack(Thread* thread, TraceData* trace_data)
   thread->SetMethodTraceBufferCurrentEntry(kAlwaysOnTraceBufSize - 1);
   trace_data->AppendToLongRunningMethods(os.str());
   trace_data->AddTracedThread(thread);
+  trace_data->AddTracedMethods(methods);
+}
+
+// Records the thread and method info.
+void DumpThreadMethodInfo(const std::unordered_map<size_t, std::string>& traced_threads,
+                          const std::unordered_set<ArtMethod*>& traced_methods,
+                          std::ostringstream& os) REQUIRES_SHARED(Locks::mutator_lock_) {
+  // Dump data about thread information.
+  os << "\n*threads\n";
+  for (const auto& it : traced_threads) {
+    os << it.first << "\t" << it.second << "\n";
+  }
+
+  // Dump data about method information.
+  os << "*methods\n";
+  for (ArtMethod* method : traced_methods) {
+    ArtMethod* method_ptr = reinterpret_cast<ArtMethod*>(method);
+    os << method_ptr << "\t" << GetMethodInfoLine(method);
+  }
+
+  os << "*end";
 }
 }  // namespace
 
@@ -168,6 +189,43 @@ class TraceStopTask : public gc::HeapTask {
   void Run([[maybe_unused]] Thread* self) override { TraceProfiler::TraceTimeElapsed(); }
 };
 
+class TraceStartCheckpoint final : public Closure {
+ public:
+  explicit TraceStartCheckpoint(LowOverheadTraceType type, TraceData* trace_data)
+      : trace_type_(type), trace_data_(trace_data), barrier_(0) {}
+
+  void Run(Thread* thread) override REQUIRES_SHARED(Locks::mutator_lock_) {
+    auto buffer = new uintptr_t[kAlwaysOnTraceBufSize];
+
+    if (trace_type_ == LowOverheadTraceType::kLongRunningMethods) {
+      // Record methods that are currently on stack.
+      RecordMethodsOnThreadStack(thread, trace_data_);
+      thread->UpdateTlsLowOverheadTraceEntrypoints(LowOverheadTraceType::kLongRunningMethods);
+    } else {
+      memset(buffer, 0, kAlwaysOnTraceBufSize * sizeof(uintptr_t));
+      thread->UpdateTlsLowOverheadTraceEntrypoints(LowOverheadTraceType::kAllMethods);
+    }
+    thread->SetMethodTraceBuffer(buffer, kAlwaysOnTraceBufSize);
+    barrier_.Pass(Thread::Current());
+  }
+
+  void WaitForThreadsToRunThroughCheckpoint(size_t threads_running_checkpoint) {
+    Thread* self = Thread::Current();
+    ScopedThreadStateChange tsc(self, ThreadState::kWaitingForCheckPointsToRun);
+    barrier_.Increment(self, threads_running_checkpoint);
+  }
+
+ private:
+  LowOverheadTraceType trace_type_;
+
+  TraceData* trace_data_;
+
+  // The barrier to be passed through and for the requestor to wait upon.
+  Barrier barrier_;
+
+  DISALLOW_COPY_AND_ASSIGN(TraceStartCheckpoint);
+};
+
 void TraceProfiler::Start(LowOverheadTraceType trace_type, uint64_t trace_duration_ns) {
   if (!art_flags::always_enable_profile_code()) {
     LOG(ERROR) << "Feature not supported. Please build with ART_ALWAYS_ENABLE_PROFILE_CODE.";
@@ -190,26 +248,16 @@ void TraceProfiler::Start(LowOverheadTraceType trace_type, uint64_t trace_durati
   profile_in_progress_ = true;
   trace_data_ = new TraceData(trace_type);
 
-  {
-    ScopedSuspendAll ssa(__FUNCTION__);
-    MutexLock tl(self, *Locks::thread_list_lock_);
-    for (Thread* thread : Runtime::Current()->GetThreadList()->GetList()) {
-      auto buffer = new uintptr_t[kAlwaysOnTraceBufSize];
-      memset(buffer, 0, kAlwaysOnTraceBufSize * sizeof(uintptr_t));
-      thread->SetMethodTraceBuffer(buffer, kAlwaysOnTraceBufSize);
-      if (trace_type == LowOverheadTraceType::kLongRunningMethods) {
-        // Record methods that are currently on stack.
-        RecordMethodsOnThreadStack(thread, trace_data_);
-        thread->UpdateTlsLowOverheadTraceEntrypoints(LowOverheadTraceType::kLongRunningMethods);
-      } else {
-        thread->UpdateTlsLowOverheadTraceEntrypoints(LowOverheadTraceType::kAllMethods);
-      }
-    }
+  Runtime* runtime = Runtime::Current();
+  TraceStartCheckpoint checkpoint(trace_type, trace_data_);
+  size_t threads_running_checkpoint = runtime->GetThreadList()->RunCheckpoint(&checkpoint);
+  if (threads_running_checkpoint != 0) {
+    checkpoint.WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint);
   }
 
   if (trace_type == LowOverheadTraceType::kLongRunningMethods) {
     // Add a Task that stops the tracing after trace_duration.
-    Runtime::Current()->GetHeap()->AddHeapTask(new TraceStopTask(NanoTime() + trace_duration_ns));
+    runtime->GetHeap()->AddHeapTask(new TraceStopTask(NanoTime() + trace_duration_ns));
     num_trace_stop_tasks_++;
   }
 }
@@ -235,7 +283,7 @@ void TraceProfiler::StopLocked() {
     return;
   }
 
-  FunctionClosure reset_buffer([](Thread* thread) {
+  static FunctionClosure reset_buffer([](Thread* thread) {
     auto buffer = thread->GetMethodTraceBuffer();
     if (buffer != nullptr) {
       delete[] buffer;
@@ -251,25 +299,7 @@ void TraceProfiler::StopLocked() {
   trace_data_ = nullptr;
 }
 
-void TraceProfiler::DumpThreadMethodInfo(
-    const std::unordered_map<size_t, std::string>& traced_threads,
-    const std::unordered_set<ArtMethod*>& traced_methods,
-    std::ostringstream& os) {
-  // Dump data about thread information.
-  os << "\n*threads\n";
-  for (const auto& it : traced_threads) {
-    os << it.first << "\t" << it.second << "\n";
-  }
-
-  // Dump data about method information.
-  os << "*methods\n";
-  for (ArtMethod* method : traced_methods) {
-    ArtMethod* method_ptr = reinterpret_cast<ArtMethod*>(method);
-    os << method_ptr << "\t" << GetMethodInfoLine(method);
-  }
 
-  os << "*end";
-}
 
 uint8_t* TraceProfiler::DumpBuffer(uint32_t thread_id,
                                    uintptr_t* method_trace_entries,
@@ -558,37 +588,53 @@ std::string TraceProfiler::GetLongRunningMethodsString() {
   return GetLongRunningMethodsStringLocked();
 }
 
+void TraceDumpCheckpoint::Run(Thread* thread) {
+  auto method_trace_entries = thread->GetMethodTraceBuffer();
+  if (method_trace_entries != nullptr) {
+    std::unordered_set<ArtMethod*> traced_methods;
+    uintptr_t* method_trace_curr_ptr = *(thread->GetTraceBufferCurrEntryPtr());
+    std::ostringstream os;
+    TraceProfiler::DumpLongRunningMethodBuffer(
+        thread->GetTid(), method_trace_entries, method_trace_curr_ptr, traced_methods, os);
+    trace_data_->AddTracedThread(thread);
+    trace_data_->AddTracedMethods(traced_methods);
+    trace_data_->AppendToLongRunningMethods(os.str());
+  }
+  barrier_.Pass(Thread::Current());
+}
+
+void TraceDumpCheckpoint::WaitForThreadsToRunThroughCheckpoint(size_t threads_running_checkpoint) {
+  Thread* self = Thread::Current();
+  ScopedThreadStateChange tsc(self, ThreadState::kWaitingForCheckPointsToRun);
+  barrier_.Increment(self, threads_running_checkpoint);
+}
+
 std::string TraceProfiler::GetLongRunningMethodsStringLocked() {
   Thread* self = Thread::Current();
   std::ostringstream os;
-  ScopedSuspendAll ssa(__FUNCTION__);
-  MutexLock tl(self, *Locks::thread_list_lock_);
-
-  // Get any data that was previously flushed.
-  const std::string& old_data = trace_data_->GetLongRunningMethods();
-  if (old_data.length() > 0) {
-    os << trace_data_->GetLongRunningMethods();
+  // Collect long running methods from all the threads;
+  Runtime* runtime = Runtime::Current();
+  TraceDumpCheckpoint checkpoint(trace_data_);
+  size_t threads_running_checkpoint = runtime->GetThreadList()->RunCheckpoint(&checkpoint);
+  if (threads_running_checkpoint != 0) {
+    checkpoint.WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint);
   }
 
-  // Collect long running methods from all the threads;
-  std::unordered_set<ArtMethod*> traced_methods;
-  for (Thread* thread : Runtime::Current()->GetThreadList()->GetList()) {
-    auto method_trace_entries = thread->GetMethodTraceBuffer();
-    if (method_trace_entries == nullptr) {
-      continue;
-    }
+  trace_data_->DumpData(os);
+  return os.str();
+}
 
-    trace_data_->AddTracedThread(thread);
-    uintptr_t* method_trace_curr_ptr = *(thread->GetTraceBufferCurrEntryPtr());
-    DumpLongRunningMethodBuffer(
-        thread->GetTid(), method_trace_entries, method_trace_curr_ptr, traced_methods, os);
+void TraceData::DumpData(std::ostringstream& os) {
+  MutexLock mu(Thread::Current(), trace_data_lock_);
+  if (long_running_methods_.length() > 0) {
+    os << long_running_methods_;
   }
 
-  trace_data_->AddTracedMethods(traced_methods);
-
   // Dump the information about traced_methods and threads
-  DumpThreadMethodInfo(trace_data_->GetTracedThreads(), trace_data_->GetTracedMethods(), os);
-  return os.str();
+  {
+    ScopedObjectAccess soa(Thread::Current());
+    DumpThreadMethodInfo(traced_threads_, traced_methods_, os);
+  }
 }
 
 void TraceProfiler::DumpLongRunningMethods(std::unique_ptr<File>&& trace_file) {
diff --git a/runtime/trace_profile.h b/runtime/trace_profile.h
index c71cb2aee6..b56ac86673 100644
--- a/runtime/trace_profile.h
+++ b/runtime/trace_profile.h
@@ -22,6 +22,8 @@
 #include "base/locks.h"
 #include "base/macros.h"
 #include "base/os.h"
+#include "thread.h"
+#include "thread_pool.h"
 
 namespace art HIDDEN {
 
@@ -43,34 +45,30 @@ enum class LowOverheadTraceType {
 
 class TraceData {
  public:
-  explicit TraceData(LowOverheadTraceType trace_type) : trace_type_(trace_type) {
-  }
+  explicit TraceData(LowOverheadTraceType trace_type)
+      : trace_type_(trace_type),
+        trace_data_lock_("Trace Data lock", LockLevel::kGenericBottomLock) {}
 
   LowOverheadTraceType GetTraceType() const {
     return trace_type_;
   }
 
-  const std::string& GetLongRunningMethods() const {
-    return long_running_methods_;
-  }
-
-  const std::unordered_set<ArtMethod*>& GetTracedMethods() const {
-    return traced_methods_;
-  }
-
-  const std::unordered_map<size_t, std::string>& GetTracedThreads() const {
-    return traced_threads_;
-  }
+  // Dumps events collected in long_running_methods_ and the information about
+  // threads and methods into the output stream.
+  void DumpData(std::ostringstream& os);
 
   void AppendToLongRunningMethods(const std::string& str) {
+    MutexLock mu(Thread::Current(), trace_data_lock_);
     long_running_methods_.append(str);
   }
 
   void AddTracedMethods(std::unordered_set<ArtMethod*>& methods) {
+    MutexLock mu(Thread::Current(), trace_data_lock_);
     traced_methods_.merge(methods);
   }
 
   void AddTracedMethod(ArtMethod* method) {
+    MutexLock mu(Thread::Current(), trace_data_lock_);
     traced_methods_.insert(method);
   }
 
@@ -79,17 +77,36 @@ class TraceData {
  private:
   // This is used to hold the initial methods on stack and also long running methods when there is a
   // buffer overflow.
-  std::string long_running_methods_;
+  std::string long_running_methods_ GUARDED_BY(trace_data_lock_);
 
   LowOverheadTraceType trace_type_;
 
   // These hold the methods and threads see so far. These are used to generate information about
   // the methods and threads.
-  std::unordered_set<ArtMethod*> traced_methods_;
+  std::unordered_set<ArtMethod*> traced_methods_ GUARDED_BY(trace_data_lock_);
 
   // Threads might exit before we dump the data, so record thread id and name when we see a new
   // thread.
-  std::unordered_map<size_t, std::string> traced_threads_;
+  std::unordered_map<size_t, std::string> traced_threads_ GUARDED_BY(trace_data_lock_);
+
+  // Lock to synchronize access to traced_methods_, traced_threads_ and long_running_methods_ which
+  // can be accessed simultaneously by multiple threads when running TraceDumpCheckpoint.
+  Mutex trace_data_lock_;
+};
+
+class TraceDumpCheckpoint final : public Closure {
+ public:
+  explicit TraceDumpCheckpoint(TraceData* trace_data) : barrier_(0), trace_data_(trace_data) {}
+
+  void Run(Thread* thread) override REQUIRES_SHARED(Locks::mutator_lock_);
+  void WaitForThreadsToRunThroughCheckpoint(size_t threads_running_checkpoint);
+
+ private:
+  // The barrier to be passed through and for the requestor to wait upon.
+  Barrier barrier_;
+
+  // Trace data to record the data from each thread.
+  TraceData* trace_data_;
 };
 
 // This class implements low-overhead tracing. This feature is available only when
@@ -178,11 +195,6 @@ class TraceProfiler {
                                           std::unordered_set<ArtMethod*>& methods /* out */,
                                           std::ostringstream& os);
 
-  // Records the thread and method info.
-  static void DumpThreadMethodInfo(const std::unordered_map<size_t, std::string>& traced_threads,
-                                   const std::unordered_set<ArtMethod*>& traced_methods,
-                                   std::ostringstream& os) REQUIRES(Locks::mutator_lock_);
-
   static bool profile_in_progress_ GUARDED_BY(Locks::trace_lock_);
 
   // Keeps track of number of outstanding trace stop tasks. We should only stop a trace when the
@@ -192,6 +204,7 @@ class TraceProfiler {
 
   static TraceData* trace_data_ GUARDED_BY(Locks::trace_lock_);
 
+  friend class TraceDumpCheckpoint;
   DISALLOW_COPY_AND_ASSIGN(TraceProfiler);
 };