[lld] r248109 - COFF: Run InputFile::parse() in background using std::async().
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 21 09:59:17 PDT 2015
On Sun, Sep 20, 2015 at 8:56 AM, Rui Ueyama <ruiu at google.com> wrote:
> 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.
>
Fair, sounds about right/likely to be OK.
the other thing to worry about would be stability of diagnostic output -
might have to cache it all and sort it or something.
>
> 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.
>
Fair enough, makes sense.
- Dave
>
>
>> - 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/20150921/a775fb93/attachment.html>
More information about the llvm-commits
mailing list