[lld] r248109 - COFF: Run InputFile::parse() in background using std::async().
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 21 16:02:52 PDT 2015
On Mon, Sep 21, 2015 at 3:58 PM, Rui Ueyama <ruiu at google.com> wrote:
> 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.
>
Hence why "nor a concurrent queue" is not quoted above my comment ;)
-- Sean Silva
>
>
>>
>> -- 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/353da9f3/attachment.html>
More information about the llvm-commits
mailing list