clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/include/lldb/Target/Trace.h:75
+ virtual llvm::Error SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) = 0;
+
----------------
This class will already have a process in "Process *m_live_process" member
variable, so no need to pass in the process. This will be NULL if we loaded a
trace from disk already, but that should be ok because it was already loaded
from something on disk.
================
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1687-1693
+ "Save the trace of the current process in the specified directory,
"
+ "which will be created if needed."
+ "This will also create a file <directory>/trace.json with the main
"
+ "properties of the trace session, along with others files which "
+ "contain the actual trace data. The trace.json file can be used "
+ "later as input for the \"trace load\" command to load the trace "
+ "in LLDB",
----------------
================
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1708
+ if (llvm::Error err =
+ trace_sp->SaveToDisk(m_options.m_directory, process_sp))
+ result.AppendError(toString(std::move(err)));
----------------
No need for the process as mentioned in above inline comment.
================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:267
}
+llvm::Expected<std::vector<uint8_t>> PostMortemThreadDecoder::GetRawTrace() {
+ FileSpec trace_file = m_trace_thread->GetTraceFile();
----------------
llvm::ArrayRef
================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:297
+
+llvm::Expected<std::vector<uint8_t>> LiveThreadDecoder::GetRawTrace() {
+ return m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());
----------------
llvm::ArrayRef
================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:40
+ /// raw trace from the thread buffer as bytes.
+ virtual llvm::Expected<std::vector<uint8_t>> GetRawTrace() = 0;
+
----------------
This data is quite large, can we just get a reference to the data without
making a copy by using llvm::ArrayRef?
================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:70
+ llvm::Expected<std::vector<uint8_t>> GetRawTrace() override;
+
----------------
llvm::ArrayRef?
================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:88
+ llvm::Expected<std::vector<uint8_t>> GetRawTrace() override;
+
----------------
llvm::ArrayRef
================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:72
+llvm::Error TraceIntelPT::SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) {
+ RefreshLiveProcessState();
----------------
remove process_sp and use "m_live_process"
================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:78
+
+llvm::Expected<std::vector<uint8_t>>
+TraceIntelPT::GetThreadBuffer(Thread &thread) {
----------------
llvm::ArrayRef
================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:25
+ llvm::Error SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) override;
+
----------------
remove process
================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:27
+
+ llvm::Expected<std::vector<uint8_t>> GetThreadBuffer(Thread &thread);
+
----------------
llvm::ArrayRef
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107669/new/
https://reviews.llvm.org/D107669
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
