russell.gallop accepted this revision.
russell.gallop added a comment.
LGTM, with a few small comments.
For the record, I wondered whether the time profiler could emit all ts as
absolute, so I tried it out. This has two problems. 1). The "Total" numbers
also need adjusting to be at the beginning of the trace time rather than zero,
which feels a little odd, and it hard to see how long things are on the time
scale. 2). All of the times end up being very large numbers so this bloats
traces considerably.
================
Comment at: clang/test/Driver/check-time-trace-sections.py:23
+
+# Make sure that the 'beginningOfTime' is not earlier than 10 seconds ago.
+if seconds_since_epoch - beginning_of_time > 10:
----------------
Could also check that beginningOfTime isn't after seconds_since_epoch.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:78
TimeTraceProfiler(unsigned TimeTraceGranularity = 0, StringRef ProcName = "")
- : StartTime(steady_clock::now()), ProcName(ProcName),
- Pid(sys::Process::getProcessId()), Tid(llvm::get_threadid()),
- TimeTraceGranularity(TimeTraceGranularity) {
+ : BeginningOfTime(system_clock::now()), StartTime(steady_clock::now()),
+ ProcName(ProcName), Pid(sys::Process::getProcessId()),
----------------
Note that this will record BeginningOfTime for each TimeTraceProfiler started
on a thread, but that won't be used. This shouldn't cause any harm and I don't
think that is very frequent so I think that this is okay.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:238
+
+ // Emit the absolute time of the moment when this TimeProfiler started
+ // measurements. This can be used to combine the profiling data from
----------------
Nit. This is a bit wordy. How about "Emit the absolute time when this
TimeProfiler started."?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78030/new/
https://reviews.llvm.org/D78030
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
