ART: Refactor GetStackTraceVisitor

Use a template to collect the stack frames. In preparation for
a fast-path avoiding the vector.

Bug: 62065987
Test: m test-art-host
Change-Id: Ib41cd07c30607393f3be33563c2c4003f002ce5b

2 files changed, 29 insertions, 48 deletions

diff --git a/runtime/openjdkjvmti/ti_stack.cc b/runtime/openjdkjvmti/ti_stack.cc
index 22da2d2f65..184fd804d0 100644
--- a/runtime/openjdkjvmti/ti_stack.cc
+++ b/runtime/openjdkjvmti/ti_stack.cc
@@ -59,13 +59,18 @@
 
 namespace openjdkjvmti {
 
+template <typename FrameFn>
 struct GetStackTraceVisitor : public art::StackVisitor {
   GetStackTraceVisitor(art::Thread* thread_in,
                        size_t start_,
-                       size_t stop_)
+                       size_t stop_,
+                       FrameFn fn_)
       : StackVisitor(thread_in, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
+        fn(fn_),
         start(start_),
         stop(stop_) {}
+  GetStackTraceVisitor(const GetStackTraceVisitor&) = default;
+  GetStackTraceVisitor(GetStackTraceVisitor&&) = default;
 
   bool VisitFrame() REQUIRES_SHARED(art::Locks::mutator_lock_) {
     art::ArtMethod* m = GetMethod();
@@ -81,7 +86,7 @@ struct GetStackTraceVisitor : public art::StackVisitor {
       jlong dex_location = (dex_pc == art::DexFile::kDexNoIndex) ? -1 : static_cast<jlong>(dex_pc);
 
       jvmtiFrameInfo info = { id, dex_location };
-      frames.push_back(info);
+      fn(info);
 
       if (stop == 1) {
         return false;  // We're done.
@@ -95,24 +100,34 @@ struct GetStackTraceVisitor : public art::StackVisitor {
     return true;
   }
 
-  std::vector<jvmtiFrameInfo> frames;
+  FrameFn fn;
   size_t start;
   size_t stop;
 };
 
-struct GetStackTraceClosure : public art::Closure {
+template <typename FrameFn>
+GetStackTraceVisitor<FrameFn> MakeStackTraceVisitor(art::Thread* thread_in,
+                                                    size_t start,
+                                                    size_t stop,
+                                                    FrameFn fn) {
+  return GetStackTraceVisitor<FrameFn>(thread_in, start, stop, fn);
+}
+
+struct GetStackTraceVectorClosure : public art::Closure {
  public:
-  GetStackTraceClosure(size_t start, size_t stop)
+  GetStackTraceVectorClosure(size_t start, size_t stop)
       : start_input(start),
         stop_input(stop),
         start_result(0),
         stop_result(0) {}
 
   void Run(art::Thread* self) OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_) {
-    GetStackTraceVisitor visitor(self, start_input, stop_input);
-    visitor.WalkStack(false);
+    auto frames_fn = [&](jvmtiFrameInfo info) {
+      frames.push_back(info);
+    };
+    auto visitor = MakeStackTraceVisitor(self, start_input, stop_input, frames_fn);
+    visitor.WalkStack(/* include_transitions */ false);
 
-    frames.swap(visitor.frames);
     start_result = visitor.start;
     stop_result = visitor.stop;
   }
@@ -220,8 +235,8 @@ jvmtiError StackUtil::GetStackTrace(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED,
     return ERR(NONE);
   }
 
-  GetStackTraceClosure closure(start_depth >= 0 ? static_cast<size_t>(start_depth) : 0,
-                               start_depth >= 0 ? static_cast<size_t>(max_frame_count) : 0);
+  GetStackTraceVectorClosure closure(start_depth >= 0 ? static_cast<size_t>(start_depth) : 0,
+                                     start_depth >= 0 ? static_cast<size_t>(max_frame_count) : 0);
   thread->RequestSynchronousCheckpoint(&closure);
 
   return TranslateFrameVector(closure.frames,
@@ -232,42 +247,6 @@ jvmtiError StackUtil::GetStackTrace(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED,
                               count_ptr);
 }
 
-struct GetAllStackTraceClosure : public art::Closure {
- public:
-  explicit GetAllStackTraceClosure(size_t stop)
-      : start_input(0),
-        stop_input(stop),
-        frames_lock("GetAllStackTraceGuard", art::LockLevel::kAbortLock),
-        start_result(0),
-        stop_result(0) {}
-
-  void Run(art::Thread* self)
-      OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_) REQUIRES(!frames_lock) {
-    // self should be live here (so it could be suspended). No need to filter.
-
-    art::Thread* current = art::Thread::Current();
-    std::vector<jvmtiFrameInfo> self_frames;
-
-    GetStackTraceVisitor visitor(self, start_input, stop_input);
-    visitor.WalkStack(false);
-
-    self_frames.swap(visitor.frames);
-
-    art::MutexLock mu(current, frames_lock);
-    frames.emplace(self, self_frames);
-  }
-
-  const size_t start_input;
-  const size_t stop_input;
-
-  art::Mutex frames_lock;
-  std::unordered_map<art::Thread*, std::vector<jvmtiFrameInfo>> frames GUARDED_BY(frames_lock);
-  size_t start_result;
-  size_t stop_result;
-};
-
-
-
 jvmtiError StackUtil::GetAllStackTraces(jvmtiEnv* env,
                                         jint max_frame_count,
                                         jvmtiStackInfo** stack_info_ptr,
@@ -300,7 +279,7 @@ jvmtiError StackUtil::GetAllStackTraces(jvmtiEnv* env,
         continue;
       }
 
-      GetStackTraceClosure closure(0u, static_cast<size_t>(max_frame_count));
+      GetStackTraceVectorClosure closure(0u, static_cast<size_t>(max_frame_count));
       thread->RequestSynchronousCheckpoint(&closure);
 
       threads.push_back(thread);
@@ -460,7 +439,7 @@ jvmtiError StackUtil::GetThreadListStackTraces(jvmtiEnv* env,
         for (size_t index = 0; index != handles.size(); ++index) {
           if (peer == handles[index].Get()) {
             // Found the thread.
-            GetStackTraceClosure closure(0u, static_cast<size_t>(max_frame_count));
+            GetStackTraceVectorClosure closure(0u, static_cast<size_t>(max_frame_count));
             thread->RequestSynchronousCheckpoint(&closure);
 
             threads.push_back(thread);
diff --git a/runtime/stack.h b/runtime/stack.h
index 8c74a8c405..fd86f5d2b1 100644
--- a/runtime/stack.h
+++ b/runtime/stack.h
@@ -532,6 +532,8 @@ class StackVisitor {
 
  public:
   virtual ~StackVisitor() {}
+  StackVisitor(const StackVisitor&) = default;
+  StackVisitor(StackVisitor&&) = default;
 
   // Return 'true' if we should continue to visit more frames, 'false' to stop.
   virtual bool VisitFrame() REQUIRES_SHARED(Locks::mutator_lock_) = 0;