[Lldb-commits] [PATCH] D105717: [trace] [intel pt] Create a "thread trace dump stats" command

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/include/lldb/Target/Trace.h:155
+  ///     If \b false, print compact stats
+  virtual void DumpTraceStats(Thread &thread, Stream &s, bool verbose) = 0;
+
----------------
Are any statistics being dumped here? Maybe DumpTraceSummary(...) or DumpTraceInfo(...) would be better?


================
Comment at: lldb/source/Commands/CommandObjectThread.cpp:2195-2197
+    LoadSubCommand(
+        "stats", CommandObjectSP(new CommandObjectTraceDumpStats(interpreter)));
   }
----------------
Since we are iterating on this new command, I am wondering if we should have just a "thread trace dump" command with options?
```
(lldb) thread trace dump --stats
(lldb) thread trace dump --instructions
```
This way the user could dump more than one thing at a time with a single command?:
```
(lldb) thread trace dump --stats --instructions
```
Just thinking out loud here, so no worries if you feel this is already correct and should stay this way



================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:118
+  }
+  s.Printf("\nraw trace size %zu\n", *raw_size);
+  return;
----------------
wallace wrote:
> the presentation of this line could be better. Something like this would look nicer
> 
>   thread 1: tid = 123123
>     
>     - Tracing technology: Intel PT
>     - Raw trace size: 1231232 bytes 
The "Tracing technology: Intel PT" should probably come before any of the thread infos if it is added:
```
Tracing technology: Intel PT
thread 1: tid = 111, size = 0x1000
thread 2: tid = 222, size = 0x1000
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105717