[Lldb-commits] [PATCH] D122093: [trace][intelpt] Added total memory usage by decoded trace

[walter erquinigo via Phabricator via lldb-commits]

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

just some few remaining nits



================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:85
+  
+  /// Get the size in bytes of non-error instance of this class
+  static size_t GetNonErrorMemoryUsage() {
----------------



================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:86-88
+  static size_t GetNonErrorMemoryUsage() {
+    return sizeof(IntelPTInstruction);
+  }
----------------
move the implementation of this this method to the corresponding cpp file. We try to keep all headers just as declarations to reduce the binary size, unless we try to achieve an optimization


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:121
+  // When adding new members to this class, make sure to update 
+  // IntelPTInstruction:: GetNonErrorMemoryUsage() if needed.
   pt_insn m_pt_insn;
----------------



================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:108-114
+  size_t mem_used = Decode(thread)->CalculateApproximateMemoryUsage();
   s.Printf("\nthread #%u: tid = %" PRIu64, thread.GetIndexID(), thread.GetID());
   if (!raw_size) {
     s.Printf(", not traced\n");
     return;
   }
   s.Printf("\n");
----------------
only calculate the memory usage if used, which happens after the early terminated if


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:119
+  s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
+           (float)mem_used / 1024);
   return;
----------------
use double instead of float. double has more precision


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122093