[PATCH] D84931: [lit] Add --time-trace-output to lit

[Julian Lettner via Phabricator via llvm-commits]

yln added a reviewer: jdenny.
yln added a comment.

That's really cool, thanks!

I am happy with this apart from some smaller nits.



================
Comment at: llvm/utils/lit/lit/reports.py:139
         return 'Unsupported configuration'
+
+class TimeTraceReport(object):
----------------
2 blank lines


================
Comment at: llvm/utils/lit/lit/reports.py:143
+        self.output_file = output_file
+        self.skipped_codes = {lit.Test.EXCLUDED,
+                              lit.Test.SKIPPED, lit.Test.UNSUPPORTED}
----------------
Unused


================
Comment at: llvm/utils/lit/lit/reports.py:146
+
+    # TODO(yln): elapsed unused, put it somewhere?
+    def write_results(self, tests, elapsed):
----------------
Please remove comment


================
Comment at: llvm/utils/lit/lit/reports.py:148
+    def write_results(self, tests, elapsed):
+        events =  [self._get_test_event(x) for x in tests]
+        first_timestamp = min(e['ts'] for e in events)
----------------
Please format the new code/use linter.
Extra space after `events =`, missing space in `#Rebase`, etc.


================
Comment at: llvm/utils/lit/lit/reports.py:152
+        for e in events:
+            e['ts'] = e['ts'] - first_timestamp
+
----------------
Probably better to get the first timestamp in the beginning and compute the desired `ts` from the start rather than rebasing everything.


================
Comment at: llvm/utils/lit/lit/reports.py:161
+        test_name = test.getFullName()
+        elapsed_time = test.result.elapsed if test.result.elapsed is not None else 0.0
+        start_time = test.result.start if test.result.start is not None else 0.0
----------------
`elapsed_time = test.result.elapsed or 0.0`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84931