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

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectThread.cpp:2195-2197
+    LoadSubCommand(
+        "stats", CommandObjectSP(new CommandObjectTraceDumpStats(interpreter)));
   }
----------------
clayborg wrote:
> 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
> 
I think that's a bit too much for this patch, but I'll keep it in mind if we end up having more dumpers.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:118
+  }
+  s.Printf("\nraw trace size %zu\n", *raw_size);
+  return;
----------------
clayborg wrote:
> 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
> ```
That's a pretty good idea.

@hanbingwang , you can invoke trace_sp->GetPluginName() for getting the name of the tracing technology being used


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105717