[lld] r248109 - COFF: Run InputFile::parse() in background using std::async().
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 20 08:56:55 PDT 2015
On Sat, Sep 19, 2015 at 8:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> 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?)?
>
I guess they are safe to be reordered, but I'm not 100% sure. This is the
place we add symbols to the symbol table for each file -- if all symbols
are distinctive, reordering the files should be totally safe. If there's a
conflict, there are many possible combinations.
{Undefined, Undefined} -- One of the two would be inserted. Because
undefined symbols have no identity, it's safe whichever we choose.
{Undefined, Defined} -- The defined symbol replaces the undefined.
{Undefined, Lazy} -- The file for the lazy symbol will be parsed.
{Lazy, Defined} -- The defined overwrites the lazy.
{Defined, Defined} -- Link failure (unless they are comdats).
All but {Lazy, Defined} seems to be okay to happen in any order. Not sure
about {Lazy, Defined}. My intuition is that that's also safe but because it
involves huge side effects, I don't know the answer.
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.
>
I think we want here is a concurrent queue rather than a function that
aggregates multiple futures. If we have such data structure, we can let
each worker to send a InputFile* to the queue when it finishes parsing a
file. Because we don't have a thread pool nor a concurrent queue, I didn't
choose to do that, but it would be probably better than this code because
it wouldn't use futures at all.
> - 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/20150920/2de1d8ec/attachment.html>
More information about the llvm-commits
mailing list