[Lldb-commits] [PATCH] D137645: [trace] Add `SBTraceCursor::GetWallClockTime` API

2022-11-08 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 474092. jj10306 added a comment. update the way items with no timestamps are handled Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137645/new/ https://reviews.llvm.org/D137645 Files:

[Lldb-commits] [PATCH] D137614: [trace] Add a new call graph reconstructor

2022-11-08 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:296 std::vector m_item_data; + std::vector m_insn_extra_info; /// The TraceItemKind for each trace item encoded as uint8_t. We don't include do we need to store

[Lldb-commits] [PATCH] D137645: [trace] Add `SBTraceCursor::GetWallClockTime` API

2022-11-08 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/source/API/SBTraceCursor.cpp:131 + const auto _wall_clock_time = m_opaque_sp->GetWallClockTime(); + return maybe_wall_clock_time ? *maybe_wall_clock_time : -1.0; +} open

[Lldb-commits] [PATCH] D137645: [trace] Add `SBTraceCursor::GetWallClockTime` API

2022-11-08 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Test Plan: Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D137509: [NFC][intelpt] Improve IntelPT trace bundle documentation

2022-11-08 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdf766fb65cc9: [NFC][intelpt] Improve IntelPT trace bundle documentation (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137509/new/

[Lldb-commits] [PATCH] D137509: [NFC][intelpt] Improve IntelPT trace bundle documentation

2022-11-07 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:244 // Optional clang/llvm target triple. + // This must be provided if the trace will be created not using the CLI or on a machine other than where

[Lldb-commits] [PATCH] D137509: [NFC][intelpt] Improve IntelPT trace bundle documentation

2022-11-07 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 473628. jj10306 marked an inline comment as done. jj10306 added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137509/new/ https://reviews.llvm.org/D137509 Files:

[Lldb-commits] [PATCH] D137509: [NFC][intelpt] Improve IntelPT trace bundle documentation

2022-11-06 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 marked an inline comment as done. jj10306 added a comment. resolve comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137509/new/ https://reviews.llvm.org/D137509 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D137509: [NFC][intelpt] Improve IntelPT trace bundle documentation

2022-11-06 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 473515. jj10306 added a comment. Improve docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137509/new/ https://reviews.llvm.org/D137509 Files:

[Lldb-commits] [PATCH] D137509: [NFC][intelpt] Improve IntelPT trace bundle documentation

2022-11-06 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Mention that the LLVM/clang triple must be provided if the trace will be

[Lldb-commits] [PATCH] D136801: [intelpt] Update Python tests to account for new errrors

2022-10-27 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG17c65e51b916: [intelpt] Update Python tests to account for new errrors (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136801/new/

[Lldb-commits] [PATCH] D136801: [intelpt] Update Python tests to account for new errrors

2022-10-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Update the Python tests (ie tests run via `lldb-dotest -p TestTrace`) to

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

2022-10-26 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf6eb089734dd: [trace][intelpt] Fix multi CPU decoding TSC assertion error (authored by jj10306). Changed prior to commit: https://reviews.llvm.org/D136610?vs=470472=470887#toc Repository: rG LLVM

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

2022-10-25 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:299-307 + uint64_t offset; + int status = pt_insn_get_offset(m_decoder_up.get(), ); + if (!IsLibiptError(status)) { +err_msg = +

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

2022-10-25 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 470472. jj10306 marked 7 inline comments as done. jj10306 added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136610/new/ https://reviews.llvm.org/D136610 Files:

[Lldb-commits] [PATCH] D136557: [trace][intel pt] Simple detection of infinite decoding loops

2022-10-24 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. looks good overall, mainly some questions and a few nits Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:123 /// of a DenseMap because

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

2022-10-24 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Occasionally the assertion that enforces increasing TSC values in

[Lldb-commits] [PATCH] D136034: [lldb][trace] Add a basic function call dump [3] - Add a JSON dumper

2022-10-17 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. nice! using this in conjunction with trace cursor we'll be able to produce some nice visualizations - lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D135917: [lldb][trace] Add a basic function call dump [2] - Implement the reconstruction algorithm

2022-10-14 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. this is awesome - the tests do a great job documenting the code and weird edge cases so thanks for that. just some minor nits and questions, looks good overall

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

2022-08-12 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:440 +Expected> lldb_private::trace_intel_pt::SplitTraceInContinuousExecutions( +TraceIntelPT

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

2022-08-11 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:164-166 + /// \param[in] decoder + /// A decoder configured to start and end within

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf9b4ea0ce9ef: [trace] Add SBTraceCursor bindings (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130930/new/

[Lldb-commits] [PATCH] D131005: [LLDB] Add SBInstruction::GetControlFlowKind()

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. jj10306 marked an inline comment as done. Closed by commit rG6cbc6e9a6d5f: [LLDB] Add SBInstruction::GetControlFlowKind() (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 449462. jj10306 added a comment. nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130930/new/ https://reviews.llvm.org/D130930 Files: lldb/bindings/interface/SBTrace.i

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 449461. jj10306 marked 4 inline comments as done. jj10306 added a comment. Revert changes to DecodedThread trace item getter API's Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130930/new/

[Lldb-commits] [PATCH] D131005: [LLDB] Add SBInstruction::GetControlFlowKind()

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. D128477 adds the control flow kind for

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:201 lldb::TraceEvent DecodedThread::GetEventByIndex(int item_index) const { + // This currently returns an undefined value when the item isn't an event. return

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. Nice work! qq: Do we plan to add this kernel tracing support for live tracing as well? Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:83 JSONTraceBundleDescription _description, ArrayRef

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 449289. jj10306 marked 4 inline comments as done. jj10306 added a comment. 1. address comments 2. rebase 3. minor changes to `DecodedThread::GetInstructionLoadAddress` and `DecodedThread::GetErrorByIndex` Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 marked 13 inline comments as done. jj10306 added inline comments. Comment at: lldb/include/lldb/API/SBTraceCursor.h:166 + /// The specific kind of event the cursor is pointing at, or \b + /// TraceEvent::eTraceEventNone if the cursor not pointing to an event. +

[Lldb-commits] [PATCH] D130925: [trace] Replace TraceCursorUP with TraceCursorSP

2022-08-01 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3bec33b16db1: [trace] Replace TraceCursorUP with TraceCursorSP (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130925/new/

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

2022-08-01 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a subscriber: mgorny. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Add bindings for the `TraceCursor` to

[Lldb-commits] [PATCH] D130924: [NFC][trace] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

2022-08-01 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9bab358e3922: [trace][intelpt] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API… (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D130925: [trace] Replace TraceCursorUP with TraceCursorSP

2022-08-01 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The use of `std::unique_ptr` with `TraceCursor` adds unnecessary complexity

[Lldb-commits] [PATCH] D130924: [trace][intelpt] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

2022-08-01 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: clayborg, wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. D130309 introduced changes to

[Lldb-commits] [PATCH] D130607: [trace] Add instruction control flow kind to JSON trace dumper's output

2022-07-27 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdde3cf2e83d2: [trace] Add instruction control flow kind to JSON trace dumpers output (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D130607: [trace] Add instruction control flow kind to JSON trace dumper's output

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: wallace, persona0220. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. D128477 adds a '-k' flag which displays

[Lldb-commits] [PATCH] D130580: Refactor string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd36ae4952d70: Add string conversion for InstructionControlFlowKind enum (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130580/new/

[Lldb-commits] [PATCH] D130580: Refactor string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 447804. jj10306 marked 3 inline comments as done. jj10306 added a comment. Remove unnecessary comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130580/new/ https://reviews.llvm.org/D130580 Files:

[Lldb-commits] [PATCH] D130580: Refactor string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 447802. jj10306 added a comment. Rebase, fix merge conflicts with D130320 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130580/new/ https://reviews.llvm.org/D130580 Files:

[Lldb-commits] [PATCH] D130580: Refactor string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Core/Disassembler.cpp:930 + } +} + Thoughts on adding a default case with a string that indicates that no string conversion was implemented for the provided variant? This would potentially make it easier

[Lldb-commits] [PATCH] D130580: Add string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Target/TraceDumper.cpp:221 _info->exe_ctx))); + +m_j.attribute( ignore this accidentally included change, will delete this and update the diff Repository: rG LLVM

[Lldb-commits] [PATCH] D130580: Add string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 447727. jj10306 added a comment. remove accidentily included change to trace dumper Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130580/new/ https://reviews.llvm.org/D130580 Files:

[Lldb-commits] [PATCH] D130580: Add string conversion for InstructionControlFlowKind enum

2022-07-26 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added reviewers: persona0220, wallace. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Refactor the string conversion of the `lldb::InstructionControlFlowKind`

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

2022-07-20 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:288 +decoded_thread.NotifyTsc(execution.thread_execution.GetLowestKnownTSC());

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

2022-07-19 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added a comment. AFAICT this doesn't handle the TSCs we get for CPU change events through the context switch traces, is that correct? This will be very useful to have for visualization purposes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

2022-07-19 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:183 -DecodedThread::TscRange::TscRange(std::map::const_iterator it,

[Lldb-commits] [PATCH] D129257: [trace][intel pt] Add a cgroup filter

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. thanks for answering those questions, lgtm Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:87-90 +std::string slice =

[Lldb-commits] [PATCH] D129340: [trace][intel pt] Create a CPU change event and expose it in the dumper

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. thanks for following up on those questions, lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129340/new/ https://reviews.llvm.org/D129340

[Lldb-commits] [PATCH] D129249: [trace][intel pt] Measure the time it takes to decode a thread in per-cpu mode

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129249/new/ https://reviews.llvm.org/D129249

[Lldb-commits] [PATCH] D129613: [trace][intel pt] Add a nice parser for the trace size

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp:174 + + std::map multipliers = {{"mib", 1024 * 1024}, +

[Lldb-commits] [PATCH] D129239: [trace] Add an option to save a compact trace bundle

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. lgtm, two minor comments Comment at: lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp:318 +should_copy = true; +} else if

[Lldb-commits] [PATCH] D129257: [trace][intel pt] Add a cgroup filter

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. looks good overall, just a couple questions from my end Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:87-90 +std::string slice =

[Lldb-commits] [PATCH] D129340: [trace][intel pt] Create a CPU change event and expose it in the dumper

2022-07-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. looks good overall, just a couple minor suggestions and questions Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:288 +

[Lldb-commits] [PATCH] D128576: [trace] Make events first class items in the trace cursor and rework errors

2022-06-29 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. This is awesome work - the code is much more understandable, so thanks for doing this! lgtm overall, just left a couple final minor comments. I think we can continue to iterate/improve

[Lldb-commits] [PATCH] D128576: [trace] Make events first class items in the trace cursor and rework errors

2022-06-28 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. review- part 1 Comment at: lldb/include/lldb/Target/Trace.h:171 + /// information failed to load, an \a llvm::Error is returned. + virtual llvm::Expected +

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

2022-06-28 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. lgtm, thanks for making the cursor traversal much cleaner Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:228-231 - // We insert a fake error signaling

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

2022-06-24 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. will take a complete look over the weekend, but wanted to point out the conflict with @persona0220's diff asap Comment at:

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

2022-06-24 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. Thanks for working on this  Looks good overall, just left some minor comments. Comment at: lldb/include/lldb/Target/TraceCursor.h:64 /// Low level traversal: -/// Unlike the \a TraceCursor::Next() API,

[Lldb-commits] [PATCH] D128484: [NFC][lldb][trace] Rename trace session to trace bundle

2022-06-24 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. lgtm - thanks for doing this renaming  Comment at: lldb/include/lldb/Core/PluginManager.h:346 - static TraceCreateInstanceForSessionFile + static

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

2022-06-22 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Target/TraceInstructionDumper.cpp:193-194 + OutputWriterJSON(Stream , const TraceInstructionDumperOptions ) + : m_s(s),

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

2022-06-22 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. looks great overall, just a couple minor comments! Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:46 /// state and granularity. class

[Lldb-commits] [PATCH] D128107: [trace] Add LoadTraceFromFile to SBDebugger and SBTrace

2022-06-20 Thread Jakob Johnson via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG50f936796072: Add LoadTraceFromFile to SBDebugger and SBTrace (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128107/new/

[Lldb-commits] [PATCH] D128107: [trace] Add LoadTraceFromFile to SBDebugger and SBTrace

2022-06-20 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py:139 +if self.USE_SB_API: +traceDescriptionFile = lldb.SBFileSpec(traceDescriptionFilePath) +loadTraceError = lldb.SBError()

[Lldb-commits] [PATCH] D128107: [trace] Add LoadTraceFromFile to SBDebugger and SBTrace

2022-06-20 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 438449. jj10306 marked 2 inline comments as done. jj10306 added a comment. rebase and use SBFileSpec constructor with that requires an explicit `resolve` flag before landing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D128107: [trace] Add LoadTraceFromFile to SBDebugger and SBTrace

2022-06-20 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/include/lldb/API/SBDebugger.h:403 + /// trace session. + SBTrace LoadTraceFromFile(SBError , const char *trace_file_path); + wallace wrote: > wallace wrote: > > JDevlieghere wrote: > > > Can this take a

[Lldb-commits] [PATCH] D128107: [trace] Add LoadTraceFromFile to SBDebugger and SBTrace

2022-06-20 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 updated this revision to Diff 438400. jj10306 marked 12 inline comments as done. jj10306 added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128107/new/ https://reviews.llvm.org/D128107 Files:

[Lldb-commits] [PATCH] D128107: Add LoadTraceFromFile to SBDebugger and SBTrace

2022-06-17 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision. jj10306 added a reviewer: wallace. Herald added a project: All. jj10306 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Add trace load functionality to SBDebugger via the `LoadTraceFromFile` method. Update

[Lldb-commits] [PATCH] D127456: [trace][intelpt] Support system-wide tracing [17] - Some improvements

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:188 + + bool was_enabled = m_enabled; if (Error err = DisableWithIoctl()) wallace wrote: >

[Lldb-commits] [PATCH] D127819: [trace][intelpt] Support system-wide tracing [21] - Support long numbers in JSON

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp:25 +json::Value toJSON(const JSONUINT64 , bool hex) { + if (hex) should there be

[Lldb-commits] [PATCH] D127817: [trace][intelpt] Support system-wide tracing [20] - Rename some fields in the schema

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/docs/lldb-gdb-remote.txt:566 // } -// -// INTEL PT why delete this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D127804: [trace][intelpt] Support system-wide tracing [19] - Some other minor improvements

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:53 +/// +/// \param[in] decoded_thread +/// All decoded instructions, errors and events will be

[Lldb-commits] [PATCH] D127752: [trace][intelpt] Support system-wide tracing [18] - some more improvements

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Herald added a subscriber: Michael137. looks great overall, just a couple minor things. Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:205-217 if

[Lldb-commits] [PATCH] D127456: [trace][intelpt] Support system-wide tracing [17] - Some improvements

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Herald added a subscriber: Michael137. Looks great overall, thanks for making these improvements - just a couple minor things Comment at:

[Lldb-commits] [PATCH] D125932: [trace][intelpt] Support system-wide tracing [10] - Return warnings and tsc information from lldb-server.

2022-06-15 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Herald added a subscriber: Michael137. sending this back to you based on the one minor thing we discussed related to reloading. If this is addressed in one of the cleanup diffs,

[Lldb-commits] [PATCH] D126394: [trace][intelpt] Support system-wide tracing [14] - Decode per cpu

2022-06-12 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:83-85 + uint64_t ToNanos(uint64_t tsc) const; + uint64_t ToTSC(uint64_t nanos)

[Lldb-commits] [PATCH] D127001: [trace][intelpt] Support system-wide tracing [16] - Create threads automatically from context switch data in the post-mortem case

2022-06-09 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp:167-170 + // A list of known threads for the given process. When

[Lldb-commits] [PATCH] D126015: [trace][intelpt] Support system-wide tracing [12] - Support multi-core trace load and save

2022-06-04 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added a comment. feeback-v3 - completed review Comment at: lldb/docs/use/intel_pt.rst:168 :: { +"type": "intel-pt", Consider adding a section on the perfTscConversion parameters while we're editing this file as I don't currently see that in

[Lldb-commits] [PATCH] D126015: [trace][intelpt] Support system-wide tracing [12] - Support multi-core trace load and save

2022-06-03 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added a comment. feedback-v2 Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:183 +Expected> +PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) { + CollectionState previous_state = m_collection_state; Do we need the

[Lldb-commits] [PATCH] D126015: [trace][intelpt] Support system-wide tracing [12] - Support multi-core trace load and save

2022-05-31 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. feedback-v1 Comment at: lldb/include/lldb/Target/Trace.h:520 + /// core id -> data kind -> size + llvm::DenseMap> + m_live_core_data;

[Lldb-commits] [PATCH] D125943: [trace][intelpt] Support system-wide tracing [11] - Read warnings and perf conversion in the client

2022-05-31 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jsji. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125943/new/ https://reviews.llvm.org/D125943

[Lldb-commits] [PATCH] D125932: [trace][intelpt] Support system-wide tracing [10] - Return warnings and tsc information from lldb-server.

2022-05-27 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:40 +IntelPTCollector::FetchPerfTscConversionParameters() { + if (!m_cached_tsc_conversion) { +if (Expected tsc_conversion = Don't we want to always reload the

[Lldb-commits] [PATCH] D126267: [trace][intelpt] Support system-wide tracing [13] - Add context switch decoding

2022-05-27 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp:22-42 +struct PerfContextSwitchRecord { + struct perf_event_header header;

[Lldb-commits] [PATCH] D125897: [trace][intelpt] Support system-wide tracing [9] - Collect and return context switch traces

2022-05-19 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:122-129 void IntelPTMultiCoreTrace::ForEachCore( std::function callback) { for (auto : m_traces_per_core) -callback(it.first, *it.second); +

[Lldb-commits] [PATCH] D125897: [trace][intelpt] Support system-wide tracing [9] - Collect and return context switch traces

2022-05-19 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Submitting my comments so far, will continue my review in a bit Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:26-28 static const char

[Lldb-commits] [PATCH] D125850: [trace][intelpt] Support system-wide tracing [8] - Improve the single buffer perf_event configuration

2022-05-19 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:186 attr.exclude_kernel = 1; - attr.sample_type = PERF_SAMPLE_TIME; -

[Lldb-commits] [PATCH] D125503: [trace][intelpt] Support system-wide tracing [7] - Create a base IntelPTProcessTrace class

2022-05-19 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:73 + /// The target process. NativeProcessProtocol _process; /// Threads traced due

[Lldb-commits] [PATCH] D124962: [trace][intelpt] Support system-wide tracing [5] - Disable/enable per-core tracing based on the process state

2022-05-17 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:211 +Error IntelPTSingleBufferTrace::SetCollectionState( +TraceCollectionState

[Lldb-commits] [PATCH] D125047: [trace][intelpt] Support system-wide tracing [6] - Break IntelPTCollector into smaller files and minor refactor

2022-05-11 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Utility/TraceGDBRemotePackets.cpp:136 {"tid", packet.tid}, -{"size", packet.size}}); +

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

2022-05-11 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:152-156 + IntelPTPerThreadProcessTraceUP m_per_thread_process_trace_up; + /// Cores traced due to per-core "process tracing". Only one active + /// "process tracing" instance is

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

2022-05-11 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. couple minor things, but looks good overall Comment at: lldb/docs/lldb-gdb-remote.txt:465-474 // "tid": , // "binaryData": [ //{ // "kind":

[Lldb-commits] [PATCH] D124962: [trace][intelpt] Support system-wide tracing [5] - Disable/enable per-core tracing based on the process state

2022-05-10 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:234 +IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, size_t size, + bool flush) { std::vector data(size, 0);

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

2022-05-10 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:228-234 + return m_per_thread_process_trace_up->TraceStart(tid); } Error IntelPTCollector::OnThreadDestroyed(lldb::tid_t tid) { - if (IsProcessTracingEnabled() &&

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

2022-05-08 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/docs/lldb-gdb-remote.txt:372-374 +// This limit applies to the sum of the sizes of all trace and core +// buffers for the current process, excluding the ones started with +// "thread tracing".

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

2022-05-08 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. I'm assuming the changes to make the single buffer trace class generic for threads and cpu buffers is done in the next diff so stamping this, feel free to update with those changes if that

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added a comment. Much cleaner now, thanks for separating out the procfs logic  Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124573/new/ https://reviews.llvm.org/D124573 ___ lldb-commits

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added inline comments. This revision now requires changes to proceed. Comment at: lldb/docs/lldb-gdb-remote.txt:498 // Binary data kinds: -//- threadTraceBuffer: trace buffer for a thread. +//- traceBuffer: trace

[Lldb-commits] [PATCH] D124640: [trace][intelpt] Support system-wide tracing [2] - Add a dummy --per-core-tracing option

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Before doing a complete review can you provide clarity on the decision to only support perCore for process wide (see my inline comment with my thoughts/questions)? Understanding

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-05-02 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added inline comments. Comment at: lldb/unittests/Process/Linux/PerfTests.cpp:53 +TEST(Perf, HardcodedLogicalCoreIDs) { + Expected> core_ids = Not directly related to this diff, but I just realized the proc fs parsing

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-04-28 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments. Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:22 +// List of data kinds used by jLLDBGetState and jLLDBGetBinaryData. +struct IntelPTDataKinds { + static const char *kProcFsCpuInfo; why not use an enum here?

  1   2   >