[Lldb-commits] [lldb] Don't count all the frames just to skip the current inlined ones. (PR #80918)

[via lldb-commits]

https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/80918

>From c76edeec5c7430cd352c4d0ca977445800c55666 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Tue, 6 Feb 2024 17:30:17 -0800
Subject: [PATCH 1/2] Don't count all the frames just to skip the current
 inlined ones.

The algorithm to find the DW_OP_entry_value requires you to find
the nearest non-inlined frame.  It did that by counting the number
of stack frames so that it could use that as a loop stopper.

That is unnecessary and inefficient.  Unnecessary because
GetFrameAtIndex will return a null frame when you step past the
oldest frame, so you already have the "got to the end" signal
without counting all the stack frames.
And counting all the stack frames can be expensive.
---
 lldb/source/Expression/DWARFExpression.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index fe4928d4f43a43..c061fd1140fff7 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -608,11 +608,10 @@ static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
   StackFrameSP parent_frame = nullptr;
   addr_t return_pc = LLDB_INVALID_ADDRESS;
   uint32_t current_frame_idx = current_frame->GetFrameIndex();
-  uint32_t num_frames = thread->GetStackFrameCount();
-  for (uint32_t parent_frame_idx = current_frame_idx + 1;
-       parent_frame_idx < num_frames; ++parent_frame_idx) {
+
+  for (uint32_t parent_frame_idx = current_frame_idx + 1;;parent_frame_idx++) {
     parent_frame = thread->GetStackFrameAtIndex(parent_frame_idx);
-    // Require a valid sequence of frames.
+    // If this is null, we're at the end of the stack.
     if (!parent_frame)
       break;
 

>From e8659a128f34b93469e9ad9b0ed013ff6764c5be Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Wed, 7 Feb 2024 11:31:09 -0800
Subject: [PATCH 2/2] Add a comment about Thread::GetStackFrameCount being
 expensive.

---
 lldb/include/lldb/Target/Thread.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index e423dd4a6d2baa..30863ad4c90299 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -390,6 +390,13 @@ class Thread : public std::enable_shared_from_this<Thread>,
   /// and having the thread call the SystemRuntime again.
   virtual bool ThreadHasQueueInformation() const { return false; }
 
+  /// GetStackFrameCount can be expensive.  Stacks can get very deep, and they
+  /// require memory reads for each frame.  So only use GetStackFrameCount when 
+  /// you need to know the depth of the stack.  When iterating over frames, its
+  /// better to generate the frames one by one with GetFrameAtIndex, and when
+  /// that returns NULL, you are at the end of the stack.  That way your loop
+  /// will only do the work it needs to, without forcing lldb to realize
+  /// StackFrames you weren't going to look at.
   virtual uint32_t GetStackFrameCount() {
     return GetStackFrameList()->GetNumFrames();
   }