[lld] r248109 - COFF: Run InputFile::parse() in background using std::async().

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 19 20:28:56 PDT 2015


On Sat, Sep 19, 2015 at 8:11 PM, Rui Ueyama via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: ruiu
> Date: Sat Sep 19 22:11:16 2015
> New Revision: 248109
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248109&view=rev
> Log:
> COFF: Run InputFile::parse() in background using std::async().
>
> Previously, InputFile::parse() was run in batch. We construct a list
> of all input files and call parse() on each file using parallel_for_each.
> That means we cannot start parsing files until we get a complete list
> of input files, although InputFile::parse() is safe to call from anywhere.
>

Do the following tasks (the loops over ArchiveQueue and ObjectQueue) need
to happen in the order of the queue contents? Or would it be correct to
handle these out of order (I think you mentioned concurrently doing this
work is currently out of the question - the concurrent symbol table is more
complexity than is worthwhile/you're investing in right now, yes?)?

If they can be correctly handled out of order you might get some more
efficiency by not blocking on the first future, but looking for the next
available future (so you can start doing work on the first available one
even if others are still being parsed - in the worst case if the first
element is the largest/longest to parse, you might be single-threaded for
some time as all the other parsing is finished and you're just waiting for
the last one)

I seem to recall Chandler bemoaning the absence of a "give me the next
available future from a colleciton of futures" in the standard library, so
this would probably involve substantially more work (& again, may not be
the right time/effort/benefit tradeoff right now) to write such tools - if
my memory serves me correctly/this hasn't been fixed in the intervening
years since I remember having that conversation.

- Dave


>
> This patch makes it asynchronous. As soon as we add a file to the symbol
> table, we now start parsing the file using std::async().
>
> This change shortens self-hosting time (650 ms) by 28 ms. It's about 4%
> improvement.
>
> Modified:
>     lld/trunk/COFF/SymbolTable.cpp
>     lld/trunk/COFF/SymbolTable.h
>
> Modified: lld/trunk/COFF/SymbolTable.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=248109&r1=248108&r2=248109&view=diff
>
> ==============================================================================
> --- lld/trunk/COFF/SymbolTable.cpp (original)
> +++ lld/trunk/COFF/SymbolTable.cpp Sat Sep 19 22:11:16 2015
> @@ -27,10 +27,12 @@ void SymbolTable::addFile(std::unique_pt
>    InputFile *File = FileP.get();
>    Files.push_back(std::move(FileP));
>    if (auto *F = dyn_cast<ArchiveFile>(File)) {
> -    ArchiveQueue.push_back(F);
> +    ArchiveQueue.push_back(
> +        std::async(std::launch::async, [=]() { F->parse(); return F; }));
>      return;
>    }
> -  ObjectQueue.push_back(File);
> +  ObjectQueue.push_back(
> +      std::async(std::launch::async, [=]() { File->parse(); return File;
> }));
>    if (auto *F = dyn_cast<ObjectFile>(File)) {
>      ObjectFiles.push_back(F);
>    } else if (auto *F = dyn_cast<BitcodeFile>(File)) {
> @@ -56,13 +58,11 @@ void SymbolTable::readArchives() {
>    if (ArchiveQueue.empty())
>      return;
>
> -  parallel_for_each(ArchiveQueue.begin(), ArchiveQueue.end(),
> -                    [](ArchiveFile *File) { File->parse(); });
> -
>    // Add lazy symbols to the symbol table. Lazy symbols that conflict
>    // with existing undefined symbols are accumulated in LazySyms.
>    std::vector<Symbol *> LazySyms;
> -  for (ArchiveFile *File : ArchiveQueue) {
> +  for (std::future<ArchiveFile *> &Future : ArchiveQueue) {
> +    ArchiveFile *File = Future.get();
>      if (Config->Verbose)
>        llvm::outs() << "Reading " << File->getShortName() << "\n";
>      for (Lazy &Sym : File->getLazySymbols())
> @@ -82,25 +82,21 @@ void SymbolTable::readObjects() {
>
>    // Add defined and undefined symbols to the symbol table.
>    std::vector<StringRef> Directives;
> -  for (size_t I = 0; I < ObjectQueue.size();) {
> -    parallel_for_each(ObjectQueue.begin() + I, ObjectQueue.end(),
> -                      [](InputFile *File) { File->parse(); });
> -    for (size_t E = ObjectQueue.size(); I != E; ++I) {
> -      InputFile *File = ObjectQueue[I];
> +  for (size_t I = 0; I < ObjectQueue.size(); ++I) {
> +    InputFile *File = ObjectQueue[I].get();
> +    if (Config->Verbose)
> +      llvm::outs() << "Reading " << File->getShortName() << "\n";
> +    // Adding symbols may add more files to ObjectQueue
> +    // (but not to ArchiveQueue).
> +    for (SymbolBody *Sym : File->getSymbols())
> +      if (Sym->isExternal())
> +        addSymbol(Sym);
> +    StringRef S = File->getDirectives();
> +    if (!S.empty()) {
> +      Directives.push_back(S);
>        if (Config->Verbose)
> -        llvm::outs() << "Reading " << File->getShortName() << "\n";
> -      // Adding symbols may add more files to ObjectQueue
> -      // (but not to ArchiveQueue).
> -      for (SymbolBody *Sym : File->getSymbols())
> -        if (Sym->isExternal())
> -          addSymbol(Sym);
> -      StringRef S = File->getDirectives();
> -      if (!S.empty()) {
> -        Directives.push_back(S);
> -        if (Config->Verbose)
> -          llvm::outs() << "Directives: " << File->getShortName()
> -                       << ": " << S << "\n";
> -      }
> +        llvm::outs() << "Directives: " << File->getShortName()
> +                     << ": " << S << "\n";
>      }
>    }
>    ObjectQueue.clear();
>
> Modified: lld/trunk/COFF/SymbolTable.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=248109&r1=248108&r2=248109&view=diff
>
> ==============================================================================
> --- lld/trunk/COFF/SymbolTable.h (original)
> +++ lld/trunk/COFF/SymbolTable.h Sat Sep 19 22:11:16 2015
> @@ -16,6 +16,13 @@
>  #include "llvm/Support/Allocator.h"
>  #include "llvm/Support/raw_ostream.h"
>
> +#ifdef _MSC_VER
> +// <future> depends on <eh.h> for __uncaught_exception.
> +#include <eh.h>
> +#endif
> +
> +#include <future>
> +
>  namespace llvm {
>  struct LTOCodeGenerator;
>  }
> @@ -104,8 +111,8 @@ private:
>    llvm::DenseMap<StringRef, Symbol *> Symtab;
>
>    std::vector<std::unique_ptr<InputFile>> Files;
> -  std::vector<ArchiveFile *> ArchiveQueue;
> -  std::vector<InputFile *> ObjectQueue;
> +  std::vector<std::future<ArchiveFile *>> ArchiveQueue;
> +  std::vector<std::future<InputFile *>> ObjectQueue;
>
>    std::vector<BitcodeFile *> BitcodeFiles;
>    std::vector<SmallVector<char, 0>> Objs;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150919/58ba746e/attachment-0001.html>


More information about the llvm-commits mailing list