[PATCH] D90686: [lld][ELF] Add additional time trace categories

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: lld/ELF/DriverUtils.cpp:242
 Optional<std::string> elf::searchLibrary(StringRef name) {
+  llvm::TimeTraceScope timeScope("Locate library", name);
   if (name.startswith(":"))
----------------
MaskRay wrote:
> This is a pretty simple operation. Is the scope needed? (If file traversal IO turns out to be an issue, this still needs more information to locate the issue.)
When digging into the new "Load files" scope for my test case, I noticed gaps between each library of fairly consistent sizes, which were caused by this function. They are relatively minor overall, but if digging into the area, it looks odd that such gaps exist. There's also a fairly easy way to avoid the cost too, by explicitly specifying the library as an input file rather than using `-l/-L` to find it. The scope states what library is being looked for, and a user can use --verbose and/or --trace to identify the specific found library, to allow them to rectify the situation.


================
Comment at: lld/ELF/Writer.cpp:207
 void elf::combineEhSections() {
+  llvm::TimeTraceScope timeScope("Combine exception sections");
   for (InputSectionBase *&s : inputSections) {
----------------
MaskRay wrote:
> EH is probably more appropriate than exception.
> 
> .gcc_except_table is not handled here.
If by EH you mean specifically .eh_frame, then that would be misleading for those using ARM exidx instead? If it means something else, I'm not sure what it means.


================
Comment at: lld/ELF/Writer.cpp:2010
+  {
+    llvm::TimeTraceScope timeScope("Finalize symbols");
+    // Now that we have defined all possible global symbols including linker-
----------------
MaskRay wrote:
> This may be ambiguous.
> 
> Symbol resolution has been done. This finalizes .symtab and part of .dynsym . You'll need to include the next block to finalize the rest of .dynsym (there is still a horrible Haxagon hack below but that is just one symbol which can be ignored from the profiling)
I can  try "Add symbols to symtab", and include the block below as well then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90686