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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
