This is an archive of the discontinued LLVM Phabricator instance.

[trace] Add an option to save a compact trace bundle
ClosedPublic

Authored by wallace on Jul 6 2022, 4:53 PM.

Details

Summary

A trace bundle contains many trace files, and, in the case of intel pt, the
largest files are often the context switch traces because they are not
compressed by default. As a way to improve this, I'm adding a --compact option
to the trace save command that filters out unwanted processes from the
context switch traces. Eventually we can do the same for intel pt traces as
well.

Diff Detail

Event Timeline

wallace created this revision.Jul 6 2022, 4:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2022, 4:53 PM
wallace requested review of this revision.Jul 6 2022, 4:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2022, 4:53 PM
wallace added inline comments.Jul 6 2022, 4:56 PM
lldb/source/Commands/CommandObjectProcess.cpp
582

many formatting changes sneaked in. The actual changes are in the end of this file

lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
241

this was a bug in the existing code that prevented copying a module that was loaded from a bundle

wallace updated this revision to Diff 442759.Jul 6 2022, 7:56 PM

minor improvements for autocompletion in the CLI

wallace updated this revision to Diff 442765.Jul 6 2022, 8:22 PM

make relative all paths being returned in the description file

wallace updated this revision to Diff 442769.Jul 6 2022, 8:41 PM

improve command handling

wallace updated this revision to Diff 442770.Jul 6 2022, 8:44 PM

fix tests

wallace updated this revision to Diff 442779.Jul 6 2022, 10:32 PM

rename 'process trace save' to 'trace save' because it can actually dump the information of multiple processes.

jj10306 accepted this revision.Jul 13 2022, 8:37 AM

lgtm, two minor comments

lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
318

why do we want the error record?

323–325

consider using std::copy or .insert here instead?

This revision is now accepted and ready to land.Jul 13 2022, 8:37 AM
JDevlieghere added inline comments.Jul 13 2022, 8:49 AM
lldb/source/Commands/CommandObjectProcess.cpp
582

Please separate the formatting changes into a separate patch/commit. That way they're much easier to identify in the git blame output instead of getting attributed to an unrelated change. What I usually do in this scenario is format the file, commit than, and then rebase my change on top.

I'll remove the unwanted formatting changes

wallace added inline comments.Jul 13 2022, 11:36 AM
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
318

i don't want to lose this information because they can show contention issues