wallace added inline comments.
================
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:19
+#include "lldb/lldb-types.h"
+#include "llvm/Analysis/ModuleSummaryAnalysis.h"
+#include "llvm/Support/Error.h"
----------------
this doesn't seem relevant
================
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:22
+#include "llvm/Support/JSON.h"
+#include <fstream>
+#include <sstream>
----------------
leave a black line before this
================
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:31
+
+llvm::Error TraceIntelPTSessionFileSaver::SaveToDisk(
+ lldb::ProcessSP &process_sp, TraceIntelPT &trace_ipt, FileSpec directory) {
----------------
Better rename this class to TraceIntelPTSessionSaver, because this is not just
working with a file, it's instead dealing with a directory of many files
================
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:56
+
+void TraceIntelPTSessionFileSaver::WriteIntelPTSchemaToFile(
+ const JSONTraceIntelPTSchema &json_trace_intel_pt_schema,
----------------
To make these useful functions more generic so that they can be reused by other
Trace technologies, you can create a file TraceSessionSaver.h/cpp in
Plugins/Trace/common.
Then you can move this function to that file an call it
WriteTraceSession(llvm::Value session_json, FileSpec directory)
================
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:87
+llvm::Expected<JSONTraceSessionBase>
+TraceIntelPTSessionFileSaver::BuildProcessesSection(lldb::ProcessSP
&process_sp,
+ TraceIntelPT &trace_ipt,
----------------
you could move BuildProcessesSection, BuildThreadsSection and
BuildModulesSection to TraceSessionSaver.h/cpp if you provide a callback that
receives a thread_id and returns a raw trace. Then almost all the code would be
reusable by other Trace plug-ins
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107669/new/
https://reviews.llvm.org/D107669
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
