[lld] r248109 - COFF: Run InputFile::parse() in background using std::async().
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 21 15:58:22 PDT 2015
On Mon, Sep 21, 2015 at 3:53 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>
> On Sun, Sep 20, 2015 at 8:56 AM, Rui Ueyama via llvm-commits <
> llvm-commits at lists.llvm.org> 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.
>>
>> 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
>>
>
> It looks like we have something in include/lld/Core/Parallel.h. No idea
> how scalable it is.
>
No, we don't have a concurrent queue in that file.
>
> -- Sean Silva
>
>
>> 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
>>>>
>>>
>>>
>>
>> _______________________________________________
>> 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/9e2f5809/attachment.html>
More information about the llvm-commits
mailing list