<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 21, 2015 at 3:58 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Mon, Sep 21, 2015 at 3:53 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Sun, Sep 20, 2015 at 8:56 AM, Rui Ueyama via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Sat, Sep 19, 2015 at 8:28 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Sat, Sep 19, 2015 at 8:11 PM, Rui Ueyama via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ruiu<br>
Date: Sat Sep 19 22:11:16 2015<br>
New Revision: 248109<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=248109&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=248109&view=rev</a><br>
Log:<br>
COFF: Run InputFile::parse() in background using std::async().<br>
<br>
Previously, InputFile::parse() was run in batch. We construct a list<br>
of all input files and call parse() on each file using parallel_for_each.<br>
That means we cannot start parsing files until we get a complete list<br>
of input files, although InputFile::parse() is safe to call from anywhere.<br></blockquote></span><div><br>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?)?<br></div></div></div></div></blockquote><div><br></div></span><div>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.</div><div><br></div><div>{Undefined, Undefined} -- One of the two would be inserted. Because undefined symbols have no identity, it's safe whichever we choose.</div><div>{Undefined, Defined} -- The defined symbol replaces the undefined.</div><div>{Undefined, Lazy} -- The file for the lazy symbol will be parsed.</div><div>{Lazy, Defined} -- The defined overwrites the lazy.</div><div>{Defined, Defined} -- Link failure (unless they are comdats).</div><div><br></div><div>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.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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)<br><br>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.<br></div></div></div></div></blockquote><div><br></div></span><div>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</div></div></div></div></blockquote><div><br></div></span><div>It looks like we have something in include/lld/Core/Parallel.h. No idea how scalable it is.</div></div></div></div></blockquote><div><br></div></div></div><div>No, we don't have a concurrent queue in that file.</div></div></div></div></blockquote><div><br></div><div>Hence why "nor a concurrent queue" is not quoted above my comment ;)</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> 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.</div><div><div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>- Dave<br> </div><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
This patch makes it asynchronous. As soon as we add a file to the symbol<br>
table, we now start parsing the file using std::async().<br>
<br>
This change shortens self-hosting time (650 ms) by 28 ms. It's about 4%<br>
improvement.<br>
<br>
Modified:<br>
lld/trunk/COFF/SymbolTable.cpp<br>
lld/trunk/COFF/SymbolTable.h<br>
<br>
Modified: lld/trunk/COFF/SymbolTable.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=248109&r1=248108&r2=248109&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=248109&r1=248108&r2=248109&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/SymbolTable.cpp (original)<br>
+++ lld/trunk/COFF/SymbolTable.cpp Sat Sep 19 22:11:16 2015<br>
@@ -27,10 +27,12 @@ void SymbolTable::addFile(std::unique_pt<br>
InputFile *File = FileP.get();<br>
Files.push_back(std::move(FileP));<br>
if (auto *F = dyn_cast<ArchiveFile>(File)) {<br>
- ArchiveQueue.push_back(F);<br>
+ ArchiveQueue.push_back(<br>
+ std::async(std::launch::async, [=]() { F->parse(); return F; }));<br>
return;<br>
}<br>
- ObjectQueue.push_back(File);<br>
+ ObjectQueue.push_back(<br>
+ std::async(std::launch::async, [=]() { File->parse(); return File; }));<br>
if (auto *F = dyn_cast<ObjectFile>(File)) {<br>
ObjectFiles.push_back(F);<br>
} else if (auto *F = dyn_cast<BitcodeFile>(File)) {<br>
@@ -56,13 +58,11 @@ void SymbolTable::readArchives() {<br>
if (ArchiveQueue.empty())<br>
return;<br>
<br>
- parallel_for_each(ArchiveQueue.begin(), ArchiveQueue.end(),<br>
- [](ArchiveFile *File) { File->parse(); });<br>
-<br>
// Add lazy symbols to the symbol table. Lazy symbols that conflict<br>
// with existing undefined symbols are accumulated in LazySyms.<br>
std::vector<Symbol *> LazySyms;<br>
- for (ArchiveFile *File : ArchiveQueue) {<br>
+ for (std::future<ArchiveFile *> &Future : ArchiveQueue) {<br>
+ ArchiveFile *File = Future.get();<br>
if (Config->Verbose)<br>
llvm::outs() << "Reading " << File->getShortName() << "\n";<br>
for (Lazy &Sym : File->getLazySymbols())<br>
@@ -82,25 +82,21 @@ void SymbolTable::readObjects() {<br>
<br>
// Add defined and undefined symbols to the symbol table.<br>
std::vector<StringRef> Directives;<br>
- for (size_t I = 0; I < ObjectQueue.size();) {<br>
- parallel_for_each(ObjectQueue.begin() + I, ObjectQueue.end(),<br>
- [](InputFile *File) { File->parse(); });<br>
- for (size_t E = ObjectQueue.size(); I != E; ++I) {<br>
- InputFile *File = ObjectQueue[I];<br>
+ for (size_t I = 0; I < ObjectQueue.size(); ++I) {<br>
+ InputFile *File = ObjectQueue[I].get();<br>
+ if (Config->Verbose)<br>
+ llvm::outs() << "Reading " << File->getShortName() << "\n";<br>
+ // Adding symbols may add more files to ObjectQueue<br>
+ // (but not to ArchiveQueue).<br>
+ for (SymbolBody *Sym : File->getSymbols())<br>
+ if (Sym->isExternal())<br>
+ addSymbol(Sym);<br>
+ StringRef S = File->getDirectives();<br>
+ if (!S.empty()) {<br>
+ Directives.push_back(S);<br>
if (Config->Verbose)<br>
- llvm::outs() << "Reading " << File->getShortName() << "\n";<br>
- // Adding symbols may add more files to ObjectQueue<br>
- // (but not to ArchiveQueue).<br>
- for (SymbolBody *Sym : File->getSymbols())<br>
- if (Sym->isExternal())<br>
- addSymbol(Sym);<br>
- StringRef S = File->getDirectives();<br>
- if (!S.empty()) {<br>
- Directives.push_back(S);<br>
- if (Config->Verbose)<br>
- llvm::outs() << "Directives: " << File->getShortName()<br>
- << ": " << S << "\n";<br>
- }<br>
+ llvm::outs() << "Directives: " << File->getShortName()<br>
+ << ": " << S << "\n";<br>
}<br>
}<br>
ObjectQueue.clear();<br>
<br>
Modified: lld/trunk/COFF/SymbolTable.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=248109&r1=248108&r2=248109&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=248109&r1=248108&r2=248109&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/SymbolTable.h (original)<br>
+++ lld/trunk/COFF/SymbolTable.h Sat Sep 19 22:11:16 2015<br>
@@ -16,6 +16,13 @@<br>
#include "llvm/Support/Allocator.h"<br>
#include "llvm/Support/raw_ostream.h"<br>
<br>
+#ifdef _MSC_VER<br>
+// <future> depends on <eh.h> for __uncaught_exception.<br>
+#include <eh.h><br>
+#endif<br>
+<br>
+#include <future><br>
+<br>
namespace llvm {<br>
struct LTOCodeGenerator;<br>
}<br>
@@ -104,8 +111,8 @@ private:<br>
llvm::DenseMap<StringRef, Symbol *> Symtab;<br>
<br>
std::vector<std::unique_ptr<InputFile>> Files;<br>
- std::vector<ArchiveFile *> ArchiveQueue;<br>
- std::vector<InputFile *> ObjectQueue;<br>
+ std::vector<std::future<ArchiveFile *>> ArchiveQueue;<br>
+ std::vector<std::future<InputFile *>> ObjectQueue;<br>
<br>
std::vector<BitcodeFile *> BitcodeFiles;<br>
std::vector<SmallVector<char, 0>> Objs;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>