[Lldb-commits] [PATCH] D120595: [trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 added inline comments.


================
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:51
+  int64_t m_time_shift;
+  int64_t m_time_zero;
+};
----------------
What is the suggested way to serialize a u64 into JSON? I thought of two approaches:
1. Store two separate u32
2. store the integer as a string

@wallace wdyt?


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:22
 #include "llvm/ADT/None.h"
+#include <memory>
 
----------------



================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:188
+  /// TSC to nano converter. nullptr if no conversion exists.
+  std::shared_ptr<TscConverter> m_tsc_converter;
 };
----------------
Not sure `TraceIntelPT` is the correct spot to cache the converter on the client side. It will depend on how this new logic is integrated with the decoders, but curious to get your initial feedback on this.


================
Comment at: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp:75-76
+  bool base_bool = o && fromJSON(value, (TraceGetStateResponse &)packet, path) && o.map("tsc_conversion", packet.tsc_converter);
+  // call perftscconverter
+  // get an instance of perftscconverter and wrap it in optional<shared<
+  return base_bool;
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120595