[PATCH] D32881: Read public symbol names for LazyObjectFiles in parallel.

[Rafael Avila de Espindola via llvm-commits]

Interesting. I will try benchmarking this tomorrow.

Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:

> ruiu created this revision.
>
> This is in the context of https://reviews.llvm.org/D32867 and
> attempts to speed up symbols from object files in archive files.
>
> Interestingly, archives without symbol tables is faster than those
> with symbol tables with this patch. I benchmarked the linker with
> the default build configuration, stripped symbol tables from archives
> in the build tree, and run the same test again. The first attempt
> to link chrome took 8.17s (average of 10 trials using the perf
> command). The second attempt took 8.04s. This is odd but seems real.
>
> Somewhat orthogonal to this patch, but that result seems to imply
> that there's room for improvements in the code to handle archive
> symbol table.
>
>
> https://reviews.llvm.org/D32881
>
> Files:
>   lld/ELF/Driver.cpp
>   lld/ELF/InputFiles.cpp
>   lld/ELF/InputFiles.h
>
>
> Index: lld/ELF/InputFiles.h
> ===================================================================
> --- lld/ELF/InputFiles.h
> +++ lld/ELF/InputFiles.h
> @@ -226,15 +226,18 @@
>    }
>  
>    template <class ELFT> void parse();
> +  void readSymbols();
>    MemoryBufferRef getBuffer();
>    InputFile *fetch();
>  
>  private:
>    std::vector<StringRef> getSymbols();
>    template <class ELFT> std::vector<StringRef> getElfSymbols();
>    std::vector<StringRef> getBitcodeSymbols();
>  
> +  llvm::Optional<std::vector<StringRef>> Symbols;
>    bool Seen = false;
> +  llvm::BumpPtrAllocator Alloc;
>  };
>  
>  // An ArchiveFile object represents a .a file.
> Index: lld/ELF/InputFiles.cpp
> ===================================================================
> --- lld/ELF/InputFiles.cpp
> +++ lld/ELF/InputFiles.cpp
> @@ -984,8 +984,11 @@
>    return createObjectFile(MBRef);
>  }
>  
> +void LazyObjectFile::readSymbols() { Symbols = getSymbols(); }
> +
>  template <class ELFT> void LazyObjectFile::parse() {
> -  for (StringRef Sym : getSymbols())
> +  assert(Symbols.hasValue());
> +  for (StringRef Sym : *Symbols)
>      Symtab<ELFT>::X->addLazyObject(Sym, *this);
>  }
>  
> @@ -1004,8 +1007,8 @@
>      uint32_t FirstNonLocal = Sec.sh_info;
>      StringRef StringTable =
>          check(Obj.getStringTableForSymtab(Sec, Sections), toString(this));
> -    std::vector<StringRef> V;
>  
> +    std::vector<StringRef> V;
>      for (const Elf_Sym &Sym : Syms.slice(FirstNonLocal))
>        if (Sym.st_shndx != SHN_UNDEF)
>          V.push_back(check(Sym.getName(StringTable), toString(this)));
> @@ -1017,6 +1020,11 @@
>  std::vector<StringRef> LazyObjectFile::getBitcodeSymbols() {
>    std::unique_ptr<lto::InputFile> Obj =
>        check(lto::InputFile::create(this->MB), toString(this));
> +
> +  // Since this function is called inside parallel_for,
> +  // we want to use per-object allocator.
> +  StringSaver Saver(Alloc);
> +
>    std::vector<StringRef> V;
>    for (const lto::InputFile::Symbol &Sym : Obj->symbols())
>      if (!Sym.isUndefined())
> Index: lld/ELF/Driver.cpp
> ===================================================================
> --- lld/ELF/Driver.cpp
> +++ lld/ELF/Driver.cpp
> @@ -319,6 +319,18 @@
>    return Default;
>  }
>  
> +// We need to call readSymbols() on each LazyObjectFile to let it
> +// construct per-file internal data structure. Do that in parallel.
> +static void readLazyObjects(ArrayRef<InputFile *> Files) {
> +  std::vector<LazyObjectFile *> V;
> +  for (InputFile *F : Files)
> +    if (auto *Lazy = dyn_cast<LazyObjectFile>(F))
> +      V.push_back(Lazy);
> +
> +  parallelForEach(V.begin(), V.end(),
> +                  [](LazyObjectFile *F) { F->readSymbols(); });
> +}
> +
>  void LinkerDriver::main(ArrayRef<const char *> ArgsArr, bool CanExitEarly) {
>    ELFOptTable Parser;
>    opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
> @@ -380,6 +392,7 @@
>    inferMachineType();
>    setConfigs();
>    checkOptions(Args);
> +  readLazyObjects(Files);
>    if (ErrorCount)
>      return;
>  
>
>
> Index: lld/ELF/InputFiles.h
> ===================================================================
> --- lld/ELF/InputFiles.h
> +++ lld/ELF/InputFiles.h
> @@ -226,15 +226,18 @@
>    }
>  
>    template <class ELFT> void parse();
> +  void readSymbols();
>    MemoryBufferRef getBuffer();
>    InputFile *fetch();
>  
>  private:
>    std::vector<StringRef> getSymbols();
>    template <class ELFT> std::vector<StringRef> getElfSymbols();
>    std::vector<StringRef> getBitcodeSymbols();
>  
> +  llvm::Optional<std::vector<StringRef>> Symbols;
>    bool Seen = false;
> +  llvm::BumpPtrAllocator Alloc;
>  };
>  
>  // An ArchiveFile object represents a .a file.
> Index: lld/ELF/InputFiles.cpp
> ===================================================================
> --- lld/ELF/InputFiles.cpp
> +++ lld/ELF/InputFiles.cpp
> @@ -984,8 +984,11 @@
>    return createObjectFile(MBRef);
>  }
>  
> +void LazyObjectFile::readSymbols() { Symbols = getSymbols(); }
> +
>  template <class ELFT> void LazyObjectFile::parse() {
> -  for (StringRef Sym : getSymbols())
> +  assert(Symbols.hasValue());
> +  for (StringRef Sym : *Symbols)
>      Symtab<ELFT>::X->addLazyObject(Sym, *this);
>  }
>  
> @@ -1004,8 +1007,8 @@
>      uint32_t FirstNonLocal = Sec.sh_info;
>      StringRef StringTable =
>          check(Obj.getStringTableForSymtab(Sec, Sections), toString(this));
> -    std::vector<StringRef> V;
>  
> +    std::vector<StringRef> V;
>      for (const Elf_Sym &Sym : Syms.slice(FirstNonLocal))
>        if (Sym.st_shndx != SHN_UNDEF)
>          V.push_back(check(Sym.getName(StringTable), toString(this)));
> @@ -1017,6 +1020,11 @@
>  std::vector<StringRef> LazyObjectFile::getBitcodeSymbols() {
>    std::unique_ptr<lto::InputFile> Obj =
>        check(lto::InputFile::create(this->MB), toString(this));
> +
> +  // Since this function is called inside parallel_for,
> +  // we want to use per-object allocator.
> +  StringSaver Saver(Alloc);
> +
>    std::vector<StringRef> V;
>    for (const lto::InputFile::Symbol &Sym : Obj->symbols())
>      if (!Sym.isUndefined())
> Index: lld/ELF/Driver.cpp
> ===================================================================
> --- lld/ELF/Driver.cpp
> +++ lld/ELF/Driver.cpp
> @@ -319,6 +319,18 @@
>    return Default;
>  }
>  
> +// We need to call readSymbols() on each LazyObjectFile to let it
> +// construct per-file internal data structure. Do that in parallel.
> +static void readLazyObjects(ArrayRef<InputFile *> Files) {
> +  std::vector<LazyObjectFile *> V;
> +  for (InputFile *F : Files)
> +    if (auto *Lazy = dyn_cast<LazyObjectFile>(F))
> +      V.push_back(Lazy);
> +
> +  parallelForEach(V.begin(), V.end(),
> +                  [](LazyObjectFile *F) { F->readSymbols(); });
> +}
> +
>  void LinkerDriver::main(ArrayRef<const char *> ArgsArr, bool CanExitEarly) {
>    ELFOptTable Parser;
>    opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
> @@ -380,6 +392,7 @@
>    inferMachineType();
>    setConfigs();
>    checkOptions(Args);
> +  readLazyObjects(Files);
>    if (ErrorCount)
>      return;
>