PatriosTheGreat updated this revision to Diff 350596.
PatriosTheGreat added a comment.

Thanks for your suggestions.
I tried both of them, however the solution to patch assert recognizer doesn’t 
solve the original performance issue. Since the performance degradation doesn’t 
come from the recognizer. It appears since the LLDB needs to unwind at least 
one stack frame for each thread in order to determine does the LLDB even need 
to run recognizers (see this line: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/Thread.cpp#L587)

The solution to select the most relevant frame at the moment when the user 
actually needs a stack frame seems to fork fine. If I understand correctly it 
can be done at the Thread::GetStackFrameAtIndex method, since to provide stack 
trace view to the user all clients need to call it for all stack frames.

One problem I see in this solution is that recognizers should be aware to not 
call Thread::GetStackFrameAtIndex for the zeroth frame (which they actually got 
as a parameter), since otherwise it will bring to the infinity recursion. See 
changes to AssertFrameRecognizer.cpp

I’ve attached this solution to the current revision for further discussion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103271/new/

https://reviews.llvm.org/D103271

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===================================================================
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -578,14 +578,8 @@
   return raw_stop_description;
 }
 
-void Thread::SelectMostRelevantFrame() {
+void Thread::SelectMostRelevantFrame(lldb::StackFrameSP &frame_sp) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
-
-  auto frames_list_sp = GetStackFrameList();
-
-  // Only the top frame should be recognized.
-  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
-
   auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
 
   if (!recognized_frame_sp) {
@@ -606,8 +600,6 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
-
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
@@ -1405,6 +1397,15 @@
   exe_ctx.SetContext(shared_from_this());
 }
 
+lldb::StackFrameSP Thread::GetStackFrameAtIndex(uint32_t idx) {
+  auto frame = GetStackFrameList()->GetFrameAtIndex(idx);
+
+  if (frame && idx == 0)
+    SelectMostRelevantFrame(frame);
+
+  return frame;
+}
+
 StackFrameListSP Thread::GetStackFrameList() {
   std::lock_guard<std::recursive_mutex> guard(m_frame_mutex);
 
Index: lldb/source/Target/AssertFrameRecognizer.cpp
===================================================================
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -118,8 +118,9 @@
 
   // Fetch most relevant frame
   for (uint32_t frame_index = 0; frame_index < frames_to_fetch; frame_index++) 
{
-    prev_frame_sp = thread_sp->GetStackFrameAtIndex(frame_index);
-
+    prev_frame_sp = frame_index == 0
+                        ? frame_sp
+                        : thread_sp->GetStackFrameAtIndex(frame_index);
     if (!prev_frame_sp) {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
       LLDB_LOG(log, "Abort Recognizer: Hit unwinding bound ({1} frames)!",
Index: lldb/include/lldb/Target/Thread.h
===================================================================
--- lldb/include/lldb/Target/Thread.h
+++ lldb/include/lldb/Target/Thread.h
@@ -218,7 +218,7 @@
 
   virtual void RefreshStateAfterStop() = 0;
 
-  void SelectMostRelevantFrame();
+  void SelectMostRelevantFrame(lldb::StackFrameSP &frame_sp);
 
   std::string GetStopDescription();
 
@@ -396,9 +396,7 @@
     return GetStackFrameList()->GetNumFrames();
   }
 
-  virtual lldb::StackFrameSP GetStackFrameAtIndex(uint32_t idx) {
-    return GetStackFrameList()->GetFrameAtIndex(idx);
-  }
+  virtual lldb::StackFrameSP GetStackFrameAtIndex(uint32_t idx);
 
   virtual lldb::StackFrameSP
   GetFrameWithConcreteFrameIndex(uint32_t unwind_idx);


Index: lldb/source/Target/Thread.cpp
===================================================================
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -578,14 +578,8 @@
   return raw_stop_description;
 }
 
-void Thread::SelectMostRelevantFrame() {
+void Thread::SelectMostRelevantFrame(lldb::StackFrameSP &frame_sp) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
-
-  auto frames_list_sp = GetStackFrameList();
-
-  // Only the top frame should be recognized.
-  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
-
   auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
 
   if (!recognized_frame_sp) {
@@ -606,8 +600,6 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
-
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
@@ -1405,6 +1397,15 @@
   exe_ctx.SetContext(shared_from_this());
 }
 
+lldb::StackFrameSP Thread::GetStackFrameAtIndex(uint32_t idx) {
+  auto frame = GetStackFrameList()->GetFrameAtIndex(idx);
+
+  if (frame && idx == 0)
+    SelectMostRelevantFrame(frame);
+
+  return frame;
+}
+
 StackFrameListSP Thread::GetStackFrameList() {
   std::lock_guard<std::recursive_mutex> guard(m_frame_mutex);
 
Index: lldb/source/Target/AssertFrameRecognizer.cpp
===================================================================
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -118,8 +118,9 @@
 
   // Fetch most relevant frame
   for (uint32_t frame_index = 0; frame_index < frames_to_fetch; frame_index++) {
-    prev_frame_sp = thread_sp->GetStackFrameAtIndex(frame_index);
-
+    prev_frame_sp = frame_index == 0
+                        ? frame_sp
+                        : thread_sp->GetStackFrameAtIndex(frame_index);
     if (!prev_frame_sp) {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
       LLDB_LOG(log, "Abort Recognizer: Hit unwinding bound ({1} frames)!",
Index: lldb/include/lldb/Target/Thread.h
===================================================================
--- lldb/include/lldb/Target/Thread.h
+++ lldb/include/lldb/Target/Thread.h
@@ -218,7 +218,7 @@
 
   virtual void RefreshStateAfterStop() = 0;
 
-  void SelectMostRelevantFrame();
+  void SelectMostRelevantFrame(lldb::StackFrameSP &frame_sp);
 
   std::string GetStopDescription();
 
@@ -396,9 +396,7 @@
     return GetStackFrameList()->GetNumFrames();
   }
 
-  virtual lldb::StackFrameSP GetStackFrameAtIndex(uint32_t idx) {
-    return GetStackFrameList()->GetFrameAtIndex(idx);
-  }
+  virtual lldb::StackFrameSP GetStackFrameAtIndex(uint32_t idx);
 
   virtual lldb::StackFrameSP
   GetFrameWithConcreteFrameIndex(uint32_t unwind_idx);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to