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

[Russell Gallop via Phabricator via llvm-commits]

russell.gallop marked an inline comment as done.
russell.gallop added a comment.

In D71060#1787251 <https://reviews.llvm.org/D71060#1787251>, @MaskRay wrote:

> I have a general question about the `llvm::TimeTraceScope timeScope("LTO");` trace sites. Shall we just use the container function name if applicable?


The clang --time-trace feature (https://aras-p.info/blog/2019/01/16/time-trace-timeline-flame-chart-profiler-for-Clang/) is intended to be helpful for users understanding what the tools are doing with their code, not (just) LLVM developers, and I think that this should have the same aim in LLD.

As such, I tried to add scopes that correspond to user visible features (e.g. LTO, GC, ICF), rather than the functions which implement them. In some places this can be tricky as scopes don't always correspond to notional blocks (e.g. clang Frontend). I would prefer to stick with the current names if possible, though there may be better places for them.



================
Comment at: lld/ELF/Driver.cpp:2014
+  {
+    llvm::TimeTraceScope timeScope("Merge input sections");
+    for (BaseCommand *base : script->sectionCommands)
----------------
MaskRay wrote:
> This comment may be misleading.
> 
> It creates MergeSyntheticSection's and does other tasks that cannot be summaries by "Merge input sections".
> 
> Probably delete the trace here. It shouldn't take a lot of time anyway.
Okay, I'll remove this.


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

https://reviews.llvm.org/D71060