[PATCH] D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2)

Anton Afanasyev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 16 03:34:28 PST 2019


anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.


================
Comment at: lld/ELF/Driver.cpp:544
+    llvm::timeTraceProfilerWrite(profilerOS);
+    llvm::timeTraceProfilerCleanup();
+  }
----------------
russell.gallop wrote:
> ruiu wrote:
> > What does this cleanup function do? If some cleanup is needed, can we run it on timeTraceProfilerWrite?
> Cleanup disables the profiler and deletes the data.
> 
> This is the design as from the original addition of the time profiler (https://reviews.llvm.org/D58675). I think it allows more flexibility (e.g. we may want to write a text report of the same data) though I don't think we use that flexibility at the moment. I'm not sure what the original reason for this was. @anton-afanasyev please can you comment on why this is and whether `timeTraceProfilerCleanup` could be combined with `timeTraceProfilerWrite`? Thanks.
Yes, that is for the flexibility in a future. We may want in follow-ups to support different `Writers` (for instance, to terminal). But you are right it could be combined with current `Writer` for now.


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

https://reviews.llvm.org/D71060





More information about the llvm-commits mailing list