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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

For code blocks, using a descriptive name seems fine. For a trace added for a whole function, I still prefer using the function name as the trace point name. I think for users who are so familiar with the linker that they know the existence of `--time-trace` and will like to investigate the bottleneck, the function names should not impair their understanding of the pass names.



================
Comment at: lld/ELF/Driver.cpp:1754
 template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
+  llvm::TimeTraceScope timeScope("Link");
   // If a -hash-style option was not given, set to a default value,
----------------
I'd prefer `"LinkerDriver::link"`.


================
Comment at: lld/ELF/MarkLive.cpp:327
 template <class ELFT> void markLive() {
+  llvm::TimeTraceScope timeScope("GC");
   // If -gc-sections is not given, no sections are removed.
----------------
russell.gallop wrote:
> MaskRay wrote:
> > Probably just reuse the function name: markLive
> As I mentioned above, I would like these to make sense to linker users. Is that okay?
> 
> Is there a better place to measure for GC?
This is the best place, though I still feel "GC" as the name is less ideal than the function name "markLive".




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

https://reviews.llvm.org/D71060