wallace added inline comments.
================
Comment at: lldb/include/lldb/Target/Trace.h:75
+ virtual llvm::Error SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) = 0;
+
----------------
clayborg wrote:
> 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.
Fair enough. Then better rename this method to SaveLiveProcessTraceToDisk or
SaveLiveTraceToDisk, and make a comment mentioning that this will return an
error if this is not tracing a live process.
================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:36
+ /// Get thr raw trace of the thread.
+ ///
----------------
thr -> the
================
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;
+
----------------
clayborg wrote:
> This data is quite large, can we just get a reference to the data without
> making a copy by using llvm::ArrayRef?
Sadly this can't work. LiveThreadDecoder::GetRawTrace() gets the raw trace from
lldb-server and puts it in a vector without any object owning it. So the actual
vector will need to be returned because no one wants to own it. It's not that
bad though, because Expected just moves the inner data without creating copies.
================
Comment at: lldb/test/API/commands/trace/TestTraceSave.py:38-43
+ self.expect("process trace save -d " +
+ os.path.join(self.getSourceDir(), "intelpt-trace",
"trace_copy_dir"))
+ # Load the trace just saved
+ self.expect("trace load -v " +
+ os.path.join(self.getSourceDir(), "intelpt-trace",
"trace_copy_dir", "trace.json"),
+ substrs=["intel-pt"])
----------------
use self.getBuildDir() instead of self.getSourceDir() for the files were you
will store data
================
Comment at: lldb/test/API/commands/trace/TestTraceSave.py:69-70
+
+ #remove <trace_copy_dir>
+ shutil.rmtree(os.path.join(self.getSourceDir(), "intelpt-trace",
"trace_copy_dir"))
----------------
you don't need to do this. Every time the test runs all the previous data will
be wiped out
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
