[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

In D85705#2241249 <https://reviews.llvm.org/D85705#2241249>, @labath wrote:

> If the format is going to be json, why not use llvm::json from `llvm/Support/JSON.h`? That's what we been migrating most of the existing stuff to already, so going using that for new code makes perfect sense.

I am fine with this if we are ok always using JSON. StructuredData, when it parses JSON, is backed by llvm/Support/JSON.h, so we are already using it. If we don't need the flexibility to use other structured data formats we can do this.



================
Comment at: lldb/include/lldb/Target/Trace.h:157
+  std::string m_settings_dir;
+  llvm::Optional<lldb_private::FileSpec> m_global_trace_file;
+  std::map<lldb::pid_t, llvm::Optional<lldb_private::FileSpec>>
----------------
I know I suggested we might need a trace file for all process, one per process, or one per thread in the JSON schema. The question is do we need to add support for this right away? Might be easier to go with one trace file for now unless IntelPT already supports each of these?


================
Comment at: lldb/source/Target/Trace.cpp:54
+    std::string raw_schema(R"({
+  "plugin": <<<plugin_schema>>>,
+  "triple": string, // llvm-triple
----------------
Can we inline the plug-ins schema right into this stream instead of putting the "<<<plugin_schema>>>" text in here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705