[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 added inline comments.


================
Comment at: lldb/include/lldb/Target/Trace.h:265
+      lldb::tid_t tid, llvm::StringRef kind,
+      std::function<llvm::Error(llvm::ArrayRef<uint8_t> data)> callback);
+
----------------
typedef the callback to be cleaner and make the intention more clear?


================
Comment at: lldb/include/lldb/Target/Trace.h:385
+  /// tid -> data kind -> file
+  std::map<lldb::tid_t, std::unordered_map<std::string, FileSpec>>
+      m_postmortem_thread_data;
----------------
Can you explain what "kind" represents and why we need the nested map? Also, I see that for live tracing we have a map for both processes and threads, why is this not the case for post mortem?


================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:271-274
+  // The libipt library does not modify the trace buffer, hence the
+  // following casts are safe.
+  config.begin = const_cast<uint8_t *>(buffer.data());
+  config.end = const_cast<uint8_t *>(buffer.data() + buffer.size());
----------------
The couple minor changes to Libiptdecoder aren't related to this diff, maybe move these to D123106.


================
Comment at: lldb/source/Target/Trace.cpp:265
+
+  MemoryBuffer &data = **trace_or_error;
+  ArrayRef<uint8_t> array_ref(
----------------
is guaranteed that the UP will be non-null? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123281/new/

https://reviews.llvm.org/D123281