[Lldb-commits] [lldb] d849959 - [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

Alisamar Husain via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 5 09:31:46 PDT 2022


Author: Alisamar Husain
Date: 2022-04-05T22:01:36+05:30
New Revision: d849959071c8478841a9e7b1bb625e00b848f1c7

URL: https://github.com/llvm/llvm-project/commit/d849959071c8478841a9e7b1bb625e00b848f1c7
DIFF: https://github.com/llvm/llvm-project/commit/d849959071c8478841a9e7b1bb625e00b848f1c7.diff

LOG: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

This is to reduce the size of the trace further and has appreciable results.

Differential Revision: https://reviews.llvm.org/D122991

Added: 
    

Modified: 
    lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
    lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
    lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
    lldb/test/API/commands/trace/TestTraceDumpInfo.py
    lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 994d068810f1e..a4bf0c725f7e6 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -35,38 +35,37 @@ void IntelPTError::log(llvm::raw_ostream &OS) const {
   OS << "error: " << libipt_error_message;
 }
 
-IntelPTInstruction::IntelPTInstruction() {
-  m_pt_insn.ip = LLDB_INVALID_ADDRESS;
-  m_pt_insn.iclass = ptic_error;
-  m_is_error = true;
+Optional<size_t> DecodedThread::GetRawTraceSize() const {
+  return m_raw_trace_size;
 }
 
-bool IntelPTInstruction::IsError() const { return m_is_error; }
-
-lldb::addr_t IntelPTInstruction::GetLoadAddress() const { return m_pt_insn.ip; }
-
-size_t IntelPTInstruction::GetMemoryUsage() {
-  return sizeof(IntelPTInstruction);
+size_t DecodedThread::GetInstructionsCount() const {
+  return m_instruction_ips.size();
 }
 
-Optional<size_t> DecodedThread::GetRawTraceSize() const {
-  return m_raw_trace_size;
+lldb::addr_t DecodedThread::GetInstructionLoadAddress(size_t insn_index) const {
+  return m_instruction_ips[insn_index];
 }
 
 TraceInstructionControlFlowType
-IntelPTInstruction::GetControlFlowType(lldb::addr_t next_load_address) const {
-  if (IsError())
+DecodedThread::GetInstructionControlFlowType(size_t insn_index) const {
+  if (IsInstructionAnError(insn_index))
     return (TraceInstructionControlFlowType)0;
 
   TraceInstructionControlFlowType mask =
       eTraceInstructionControlFlowTypeInstruction;
 
-  switch (m_pt_insn.iclass) {
+  lldb::addr_t load_address = m_instruction_ips[insn_index];
+  uint8_t insn_byte_size = m_instruction_sizes[insn_index];
+  pt_insn_class iclass = m_instruction_classes[insn_index];
+
+  switch (iclass) {
   case ptic_cond_jump:
   case ptic_jump:
   case ptic_far_jump:
     mask |= eTraceInstructionControlFlowTypeBranch;
-    if (m_pt_insn.ip + m_pt_insn.size != next_load_address)
+    if (insn_index + 1 < m_instruction_ips.size() &&
+        load_address + insn_byte_size != m_instruction_ips[insn_index + 1])
       mask |= eTraceInstructionControlFlowTypeTakenBranch;
     break;
   case ptic_return:
@@ -92,14 +91,16 @@ void DecodedThread::RecordTscForLastInstruction(uint64_t tsc) {
     // get a first valid TSC not in position 0. We can safely force these error
     // instructions to use the first valid TSC, so that all the trace has TSCs.
     size_t start_index =
-        m_instruction_timestamps.empty() ? 0 : m_instructions.size() - 1;
+        m_instruction_timestamps.empty() ? 0 : m_instruction_ips.size() - 1;
     m_instruction_timestamps.emplace(start_index, tsc);
     m_last_tsc = tsc;
   }
 }
 
 void DecodedThread::AppendInstruction(const pt_insn &insn) {
-  m_instructions.emplace_back(insn);
+  m_instruction_ips.emplace_back(insn.ip);
+  m_instruction_sizes.emplace_back(insn.size);
+  m_instruction_classes.emplace_back(insn.iclass);
 }
 
 void DecodedThread::AppendInstruction(const pt_insn &insn, uint64_t tsc) {
@@ -108,8 +109,10 @@ void DecodedThread::AppendInstruction(const pt_insn &insn, uint64_t tsc) {
 }
 
 void DecodedThread::AppendError(llvm::Error &&error) {
-  m_errors.try_emplace(m_instructions.size(), toString(std::move(error)));
-  m_instructions.emplace_back();
+  m_errors.try_emplace(m_instruction_ips.size(), toString(std::move(error)));
+  m_instruction_ips.emplace_back(LLDB_INVALID_ADDRESS);
+  m_instruction_sizes.emplace_back(0);
+  m_instruction_classes.emplace_back(pt_insn_class::ptic_unknown);
 }
 
 void DecodedThread::AppendError(llvm::Error &&error, uint64_t tsc) {
@@ -130,10 +133,6 @@ const DecodedThread::LibiptErrors &DecodedThread::GetTscErrors() const {
   return m_tsc_errors;
 }
 
-ArrayRef<IntelPTInstruction> DecodedThread::GetInstructions() const {
-  return makeArrayRef(m_instructions);
-}
-
 Optional<DecodedThread::TscRange>
 DecodedThread::CalculateTscRange(size_t insn_index) const {
   auto it = m_instruction_timestamps.upper_bound(insn_index);
@@ -144,7 +143,7 @@ DecodedThread::CalculateTscRange(size_t insn_index) const {
 }
 
 bool DecodedThread::IsInstructionAnError(size_t insn_idx) const {
-  return m_instructions[insn_idx].IsError();
+  return m_instruction_ips[insn_idx] == LLDB_INVALID_ADDRESS;
 }
 
 const char *DecodedThread::GetErrorByInstructionIndex(size_t insn_idx) {
@@ -167,13 +166,16 @@ void DecodedThread::SetRawTraceSize(size_t size) { m_raw_trace_size = size; }
 lldb::TraceCursorUP DecodedThread::GetCursor() {
   // We insert a fake error signaling an empty trace if needed becasue the
   // TraceCursor requires non-empty traces.
-  if (m_instructions.empty())
+  if (m_instruction_ips.empty())
     AppendError(createStringError(inconvertibleErrorCode(), "empty trace"));
   return std::make_unique<TraceCursorIntelPT>(m_thread_sp, shared_from_this());
 }
 
 size_t DecodedThread::CalculateApproximateMemoryUsage() const {
-  return IntelPTInstruction::GetMemoryUsage() * m_instructions.size() +
+  return sizeof(pt_insn::ip) * m_instruction_ips.size() +
+         sizeof(pt_insn::size) * m_instruction_sizes.size() +
+         sizeof(pt_insn::iclass) * m_instruction_classes.size() +
+         (sizeof(size_t) + sizeof(uint64_t)) * m_instruction_timestamps.size() +
          m_errors.getMemorySize();
 }
 
@@ -183,7 +185,7 @@ DecodedThread::TscRange::TscRange(std::map<size_t, uint64_t>::const_iterator it,
   auto next_it = m_it;
   ++next_it;
   m_end_index = (next_it == m_decoded_thread->m_instruction_timestamps.end())
-                    ? m_decoded_thread->GetInstructions().size() - 1
+                    ? m_decoded_thread->GetInstructionsCount() - 1
                     : next_it->first - 1;
 }
 

diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 030fea0b2fca9..f558fd4a8def6 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -51,61 +51,6 @@ class IntelPTError : public llvm::ErrorInfo<IntelPTError> {
   lldb::addr_t m_address;
 };
 
-/// \class IntelPTInstruction
-/// An instruction obtained from decoding a trace. It is either an actual
-/// instruction or an error indicating a gap in the trace.
-///
-/// Gaps in the trace can come in a few flavors:
-///   - tracing gaps (e.g. tracing was paused and then resumed)
-///   - tracing errors (e.g. buffer overflow)
-///   - decoding errors (e.g. some memory region couldn't be decoded)
-/// As mentioned, any gap is represented as an error in this class.
-class IntelPTInstruction {
-public:
-  IntelPTInstruction(const pt_insn &pt_insn)
-      : m_pt_insn(pt_insn), m_is_error(false) {}
-
-  /// Error constructor
-  IntelPTInstruction();
-
-  /// Check if this object represents an error (i.e. a gap).
-  ///
-  /// \return
-  ///     Whether this object represents an error.
-  bool IsError() const;
-
-  /// \return
-  ///     The instruction pointer address, or \a LLDB_INVALID_ADDRESS if it is
-  ///     an error.
-  lldb::addr_t GetLoadAddress() const;
-
-  /// Get the size in bytes of an instance of this class
-  static size_t GetMemoryUsage();
-
-  /// Get the \a lldb::TraceInstructionControlFlowType categories of the
-  /// instruction.
-  ///
-  /// \param[in] next_load_address
-  ///     The address of the next instruction in the trace or \b
-  ///     LLDB_INVALID_ADDRESS if not available.
-  ///
-  /// \return
-  ///     The control flow categories, or \b 0 if the instruction is an error.
-  lldb::TraceInstructionControlFlowType
-  GetControlFlowType(lldb::addr_t next_load_address) const;
-
-  IntelPTInstruction(IntelPTInstruction &&other) = default;
-
-private:
-  IntelPTInstruction(const IntelPTInstruction &other) = delete;
-  const IntelPTInstruction &operator=(const IntelPTInstruction &other) = delete;
-
-  // When adding new members to this class, make sure to update
-  // IntelPTInstruction::GetMemoryUsage() if needed.
-  pt_insn m_pt_insn;
-  bool m_is_error;
-};
-
 /// \class DecodedThread
 /// Class holding the instructions and function call hierarchy obtained from
 /// decoding a trace, as well as a position cursor used when reverse debugging
@@ -178,14 +123,27 @@ class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
   /// Append a decoding error (i.e. an instruction that failed to be decoded).
   void AppendError(llvm::Error &&error);
 
-  /// Get the instructions from the decoded trace. Some of them might indicate
-  /// errors (i.e. gaps) in the trace. For an instruction error, you can access
-  /// its underlying error message with the \a GetErrorByInstructionIndex()
-  /// method.
+  /// Append a decoding error with a corresponding TSC.
+  void AppendError(llvm::Error &&error, uint64_t tsc);
+
+  /// Get the total number of instruction pointers from the decoded trace.
+  /// This will include instructions that indicate errors (or gaps) in the
+  /// trace. For an instruction error, you can access its underlying error
+  /// message with the \a GetErrorByInstructionIndex() method.
+  size_t GetInstructionsCount() const;
+
+  /// \return
+  ///     The load address of the instruction at the given index, or \a
+  ///     LLDB_INVALID_ADDRESS if it is an error.
+  lldb::addr_t GetInstructionLoadAddress(size_t insn_index) const;
+
+  /// Get the \a lldb::TraceInstructionControlFlowType categories of the
+  /// instruction.
   ///
   /// \return
-  ///   The instructions of the trace.
-  llvm::ArrayRef<IntelPTInstruction> GetInstructions() const;
+  ///     The control flow categories, or \b 0 if the instruction is an error.
+  lldb::TraceInstructionControlFlowType
+  GetInstructionControlFlowType(size_t insn_index) const;
 
   /// Construct the TSC range that covers the given instruction index.
   /// This operation is O(logn) and should be used sparingly.
@@ -204,17 +162,6 @@ class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
   ///   points to a valid instruction.
   const char *GetErrorByInstructionIndex(size_t ins_idx);
 
-  /// Append a decoding error with a corresponding TSC.
-  void AppendError(llvm::Error &&error, uint64_t TSC);
-
-  /// Record an error decoding a TSC timestamp.
-  ///
-  /// See \a GetTscErrors() for more documentation.
-  ///
-  /// \param[in] libipt_error_code
-  ///   An error returned by the libipt library.
-  void RecordTscError(int libipt_error_code);
-
   /// Get a new cursor for the decoded thread.
   lldb::TraceCursorUP GetCursor();
 
@@ -235,6 +182,14 @@ class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
   ///   The number of TSC decoding errors.
   const LibiptErrors &GetTscErrors() const;
 
+  /// Record an error decoding a TSC timestamp.
+  ///
+  /// See \a GetTscErrors() for more documentation.
+  ///
+  /// \param[in] libipt_error_code
+  ///   An error returned by the libipt library.
+  void RecordTscError(int libipt_error_code);
+
   /// The approximate size in bytes used by this instance,
   /// including all the already decoded instructions.
   size_t CalculateApproximateMemoryUsage() const;
@@ -251,7 +206,12 @@ class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
   lldb::ThreadSP m_thread_sp;
   /// The low level storage of all instruction addresses. Each instruction has
   /// an index in this vector and it will be used in other parts of the code.
-  std::vector<IntelPTInstruction> m_instructions;
+  std::vector<lldb::addr_t> m_instruction_ips;
+  /// The size in bytes of each instruction.
+  std::vector<uint8_t> m_instruction_sizes;
+  /// The libipt instruction class for each instruction.
+  std::vector<pt_insn_class> m_instruction_classes;
+
   /// This map contains the TSCs of the decoded instructions. It maps
   /// `instruction index -> TSC`, where `instruction index` is the first index
   /// at which the mapped TSC appears. We use this representation because TSCs

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
index 59b0cc1865d1b..d9a52219d0d6f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -20,14 +20,14 @@ using namespace llvm;
 TraceCursorIntelPT::TraceCursorIntelPT(ThreadSP thread_sp,
                                        DecodedThreadSP decoded_thread_sp)
     : TraceCursor(thread_sp), m_decoded_thread_sp(decoded_thread_sp) {
-  assert(!m_decoded_thread_sp->GetInstructions().empty() &&
+  assert(m_decoded_thread_sp->GetInstructionsCount() > 0 &&
          "a trace should have at least one instruction or error");
-  m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_pos = m_decoded_thread_sp->GetInstructionsCount() - 1;
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
-  return m_decoded_thread_sp->GetInstructions().size();
+  return m_decoded_thread_sp->GetInstructionsCount();
 }
 
 bool TraceCursorIntelPT::Next() {
@@ -78,8 +78,8 @@ size_t TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) {
       return std::abs(dist);
     }
   };
-  
-	int64_t dist = FindDistanceAndSetPos();
+
+  int64_t dist = FindDistanceAndSetPos();
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
   return dist;
 }
@@ -93,7 +93,7 @@ const char *TraceCursorIntelPT::GetError() {
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
+  return m_decoded_thread_sp->GetInstructionLoadAddress(m_pos);
 }
 
 Optional<uint64_t>
@@ -109,10 +109,5 @@ TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
 
 TraceInstructionControlFlowType
 TraceCursorIntelPT::GetInstructionControlFlowType() {
-  lldb::addr_t next_load_address =
-      m_pos + 1 < GetInternalInstructionSize()
-          ? m_decoded_thread_sp->GetInstructions()[m_pos + 1].GetLoadAddress()
-          : LLDB_INVALID_ADDRESS;
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetControlFlowType(
-      next_load_address);
+  return m_decoded_thread_sp->GetInstructionControlFlowType(m_pos);
 }

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index c40b60435ff3d..8c4468a72b2db 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -112,7 +112,7 @@ void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
   }
   s << "\n";
   DecodedThreadSP decoded_trace_sp = Decode(thread);
-  size_t insn_len = decoded_trace_sp->GetInstructions().size();
+  size_t insn_len = decoded_trace_sp->GetInstructionsCount();
   size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
   s.Format("  Raw trace size: {0} KiB\n", *raw_size / 1024);

diff  --git a/lldb/test/API/commands/trace/TestTraceDumpInfo.py b/lldb/test/API/commands/trace/TestTraceDumpInfo.py
index 74b8eb38f023a..1e26ed44c1678 100644
--- a/lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ b/lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,7 +40,7 @@ def testDumpRawTraceSize(self):
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.98 KiB
-  Average memory usage per instruction: 48.00 bytes
+  Total approximate memory usage: 0.27 KiB
+  Average memory usage per instruction: 13.00 bytes
 
   Number of TSC decoding errors: 0'''])

diff  --git a/lldb/test/API/commands/trace/TestTraceLoad.py b/lldb/test/API/commands/trace/TestTraceLoad.py
index 95f57b4018276..af55462c25a0c 100644
--- a/lldb/test/API/commands/trace/TestTraceLoad.py
+++ b/lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,10 @@ def testLoadTrace(self):
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.98 KiB
-  Average memory usage per instruction: 48.00 bytes'''])
+  Total approximate memory usage: 0.27 KiB
+  Average memory usage per instruction: 13.00 bytes
+
+  Number of TSC decoding errors: 0'''])
 
     def testLoadInvalidTraces(self):
         src_dir = self.getSourceDir()


        


More information about the lldb-commits mailing list