[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added subscribers: vsk, teemperor.
teemperor added a comment.

+Vedant who had/has some plans with the statistics command at some point IIRC.

FWIW, I wanted to throw away the current statistics implementation for quite a while but I didn't have anything to replace it with. If we can replace it with this then that would be nice.



================
Comment at: lldb/include/lldb/Core/Module.h:940
+  /// ElapsedTime
+  double &GetSymtabParseTime() { return m_symtab_parse_time; }
+  double &GetSymtabNamesTime() { return m_symtab_names_time; }
----------------
Could the `double` and `uint64_t` here/below be a typedef to some unit? `typedef double Seconds`/`typedef uint64_t ByteCount` or something along those lines. Would make the unit we're expecting more obvious and I can easily find all the related code.


================
Comment at: lldb/test/API/commands/target/metrics/TestTargetMetrics.py:89
+        self.verify_key_presence(metrics, '"metrics"', keys_exist, keys_missing)
+        self.assertTrue(metrics['targetCreateTime'] >  0.0)
+        self.assertTrue(metrics['totalBreakpointResolveTime'] == 0.0)
----------------
We stopped using `assertTrue` unless there is really no better `assert*` alternative as it doesn't produce any error diagnostics on failure. There's a good overview of the replacement methods [here](https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertAlmostEqual) which all print nice errors.


================
Comment at: lldb/test/API/commands/target/metrics/TestTargetMetrics.py:199
+        """
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create  " + exe, CURRENT_EXECUTABLE_SET)
----------------
This and the following line can be just `target = self.createTestTarget()` (here and in the rest of the test).


================
Comment at: lldb/test/API/commands/target/metrics/main.cpp:2
+extern int a_function();
+extern int b_function();
+
----------------
Unused function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110804