[Lldb-commits] [PATCH] D33426: Introduce new command: thread backtrace unique

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We should probably store the stacks as lldb::addr_t values that are load addresses for quicker comparisons and searches. Inlined code details things more clearly.



================
Comment at: source/Commands/CommandObjectThread.cpp:76
+    std::vector<uint32_t> m_thread_index_ids;
+    std::stack<Address> m_stack_frames;
+  };
----------------
Might be more efficient to store a "std::stack<lldb::addr_t>" where you store the load address of each address. The comparisons would go quicker, not to mention the construction and destruction of stack itself.


================
Comment at: source/Commands/CommandObjectThread.cpp:146
+      // Iterate over threads, finding unique stack buckets.
+      std::vector<UniqueStack> unique_stacks;
+      for (const lldb::tid_t &tid : tids) {
----------------
Add an operator function for UniqueStack that only compares the stack frames:

```
bool inline operator <(const UniqueStack &lhs, const UniqueStack &rhs) {
  return lhs.m_stack_frames < rhs.m_stack_frames;
}
```
The comparison will be cheap when m_stack_frames is a std::stack of lldb::addr_t values.

Then store as a std::set for efficient lookup later instead of a linear search.


================
Comment at: source/Commands/CommandObjectThread.cpp:204
+  bool BucketThread(lldb::tid_t tid,
+                    std::vector<UniqueStack>& unique_stacks,
+                    CommandReturnObject &result) {
----------------
```
std::set<UniqueSet> &unique_stacks;
```


================
Comment at: source/Commands/CommandObjectThread.cpp:220-221
+      const lldb::StackFrameSP frame_sp = thread->GetStackFrameAtIndex(frame_index);
+      const Address frame_address = frame_sp->GetFrameCodeAddress();
+      stack_frames.push(frame_address);
+    }
----------------
```
const lldb::addr_t pc = frame_sp->GetStackID().GetPC();
stack_frames.push(pc);
```


================
Comment at: source/Commands/CommandObjectThread.cpp:226
+    bool found_match = false;
+    for (UniqueStack& unique_stack : unique_stacks) {
+      if (unique_stack.IsEqual(stack_frames)) {
----------------
use fast std::set.find() so search for the matching value.


https://reviews.llvm.org/D33426