[Lldb-commits] [PATCH] D127001: [trace][intelpt] Support system-wide tracing [16] - Create threads automatically from context switch data in the post-mortem case

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 requested changes to this revision.
jj10306 added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp:167-170
+          // A list of known threads for the given process. When context switch
+          // data is provided, LLDB will automatically create threads for the
+          // this process whenever it finds new threads when traversing the
+          // context switches.
----------------
So should this be an empty list or will the key not exist at all in the case that context switches are provided?


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp:212-215
+  "dontCreateThreadsFromContextSwitches"?: boolean,
+    // If this is true, then the automatic creation of threads from context switch
+    // data is disabled, and thus only the threads provided in the "processes.threads"
+    // section will be created.
----------------
is this necessary? can't this be inferred by if the threads key isn't present/the list is empty?
also, is this even being used rn? It appears to me that you always invoke `CreateThreadsFromContextSwitches()` if `session.cores` is present?


================
Comment at: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json:31-35
+      "threads": [
+        {
+          "tid": 3497234
+        }
+      ]
----------------
shouldn't this be gone or just an empty list depending on how we want to implement it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127001