[Lldb-commits] [PATCH] D107674: [tests] [trace] Add more comprehensive for `thread trace export ctf` command

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Thank you, this is much more understandable



================
Comment at: lldb/docs/htr.rst:1
 Hierarchical Trace Representation (HTR)
 ======================================
----------------
I'd be good to put these documentation files next in source/Plugins/TraceExporter/docs so that the documentation can live next to the source code. It'll give more visibility to this doc


================
Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:396
         {"name", display_name},
-        {"ph", "B"},
+        {"ph", "X"},
         {"ts", (int64_t)i},
----------------
a little comment explaining what "ph", "X", "dur" mean would be good. Also that in the absence of timestamps "ts" is just an index. You can repeat the comments you have in the test, btw




================
Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:424
 
+      auto duration = metadata.GetNumInstructions();
       layers_as_json.emplace_back(llvm::json::Object{
----------------
explain why the duration is the num of instructions in the case of no timestamps


================
Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:433
       });
-      start_ts = end_ts;
+      start_ts += duration ;
     }
----------------
in your devserver you can run 
"arc lint"
and it will fix all these formatting issues for you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107674