<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Jun 2, 2017 at 10:49 AM Teresa Johnson <<a href="mailto:tejohnson@google.com">tejohnson@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Cool, thanks! Just for my own edification - I assume all but the template Functor change are independent of this patch (i.e. they would have applied before)?</div></blockquote><div><br>Yeah, look like. This just caught my attention since you'd found this piece of code to be hot (obviously not so hot now - so my changes here are likely dwarfed by your fix anyway).<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> Is it worth doing the same template change to forAllSummaries?</div></blockquote><div><br>Where is that function?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Teresa</div></div><div class="gmail_extra"></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 2, 2017 at 10:25 AM, 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"><div class="gmail_quote"><div dir="ltr">Added a few cleanups/hopefully improvements in r304566 (use of SmallVector::assign to potentially streamline byte copies, use of a template to avoid std::function overhead, StringRefizing & some other little bits of tidyup). Please take a look & let me know if you've any questions/criticisms/ideas :)<br><br>- Dave<div><div class="m_3069575451471421879h5"><br><br>On Thu, Jun 1, 2017 at 6:56 PM Teresa Johnson via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div></div></div><div><div class="m_3069575451471421879h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: tejohnson<br>
Date: Thu Jun 1 20:56:02 2017<br>
New Revision: 304516<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=304516&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=304516&view=rev</a><br>
Log:<br>
[ThinLTO] Efficiency improvement when writing module path string table<br>
<br>
Summary:<br>
When writing the combined index, we are walking the entire module<br>
path StringMap in the full index, and checking whether each one should be<br>
included in the index being written. For distributed backends, where we<br>
write an individual combined index for each file, each with only a few<br>
module paths, this is incredibly inefficient. Add a method that takes<br>
a callback and hides the details of whether we are writing the full<br>
combined index, or just a slice, and in the latter case it walks the set<br>
of modules to include instead of the entire index.<br>
<br>
For a huge application with around 23K files (i.e. where we were iterating<br>
through the 23K-entry modulePath StringMap 23K times), this change improved<br>
the thin link time by a whopping 48%.<br>
<br>
Reviewers: pcc<br>
<br>
Subscribers: Prazek, inglorion, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D33813" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33813</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp<br>
<br>
Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=304516&r1=304515&r2=304516&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=304516&r1=304515&r2=304516&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)<br>
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Thu Jun 1 20:56:02 2017<br>
@@ -363,6 +363,32 @@ public:<br>
}<br>
}<br>
<br>
+ /// Calls the callback for each entry in the modulePaths StringMap that<br>
+ /// should be written to the module path string table. This hides the details<br>
+ /// of whether they are being pulled from the entire index or just those in a<br>
+ /// provided ModuleToSummariesForIndex map.<br>
+ void<br>
+ forEachModule(std::function<<br>
+ void(const StringMapEntry<std::pair<uint64_t, ModuleHash>> &)><br>
+ Callback) {</blockquote></div></div></div><div><div class="m_3069575451471421879h5"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ if (ModuleToSummariesForIndex) {<br>
+ for (const auto &M : *ModuleToSummariesForIndex) {<br>
+ const auto &MPI = Index.modulePaths().find(M.first);<br>
+ if (MPI == Index.modulePaths().end()) {<br>
+ // This should only happen if the bitcode file was empty, in which<br>
+ // case we shouldn't be importing (the ModuleToSummariesForIndex<br>
+ // would only include the module we are writing and index for).<br>
+ assert(ModuleToSummariesForIndex->size() == 1);<br>
+ continue;<br>
+ }<br>
+ Callback(*MPI);<br>
+ }<br>
+ } else {<br>
+ for (const auto &MPSE : Index.modulePaths())<br>
+ Callback(MPSE);<br>
+ }<br>
+ }<br>
+<br>
/// Main entry point for writing a combined index to bitcode.<br>
void write();<br>
<br>
@@ -370,14 +396,6 @@ private:<br>
void writeModStrings();<br>
void writeCombinedGlobalValueSummary();<br>
<br>
- /// Indicates whether the provided \p ModulePath should be written into<br>
- /// the module string table, e.g. if full index written or if it is in<br>
- /// the provided subset.<br>
- bool doIncludeModule(StringRef ModulePath) {<br>
- return !ModuleToSummariesForIndex ||<br>
- ModuleToSummariesForIndex->count(ModulePath);<br>
- }<br>
-<br>
Optional<unsigned> getValueId(GlobalValue::GUID ValGUID) {<br>
auto VMI = GUIDToValueIdMap.find(ValGUID);<br>
if (VMI == GUIDToValueIdMap.end())<br>
@@ -3149,41 +3167,41 @@ void IndexBitcodeWriter::writeModStrings<br>
unsigned AbbrevHash = Stream.EmitAbbrev(std::move(Abbv));<br>
<br>
SmallVector<unsigned, 64> Vals;<br>
- for (const auto &MPSE : Index.modulePaths()) {<br>
- if (!doIncludeModule(MPSE.getKey()))<br>
- continue;<br>
- StringEncoding Bits =<br>
- getStringEncoding(MPSE.getKey().data(), MPSE.getKey().size());<br>
- unsigned AbbrevToUse = Abbrev8Bit;<br>
- if (Bits == SE_Char6)<br>
- AbbrevToUse = Abbrev6Bit;<br>
- else if (Bits == SE_Fixed7)<br>
- AbbrevToUse = Abbrev7Bit;<br>
-<br>
- Vals.push_back(MPSE.getValue().first);<br>
-<br>
- for (const auto P : MPSE.getKey())<br>
- Vals.push_back((unsigned char)P);<br>
-<br>
- // Emit the finished record.<br>
- Stream.EmitRecord(bitc::MST_CODE_ENTRY, Vals, AbbrevToUse);<br>
-<br>
- Vals.clear();<br>
- // Emit an optional hash for the module now<br>
- auto &Hash = MPSE.getValue().second;<br>
- bool AllZero = true; // Detect if the hash is empty, and do not generate it<br>
- for (auto Val : Hash) {<br>
- if (Val)<br>
- AllZero = false;<br>
- Vals.push_back(Val);<br>
- }<br>
- if (!AllZero) {<br>
- // Emit the hash record.<br>
- Stream.EmitRecord(bitc::MST_CODE_HASH, Vals, AbbrevHash);<br>
- }<br>
+ forEachModule(<br>
+ [&](const StringMapEntry<std::pair<uint64_t, ModuleHash>> &MPSE) {<br>
+ StringEncoding Bits =<br>
+ getStringEncoding(MPSE.getKey().data(), MPSE.getKey().size());<br>
+ unsigned AbbrevToUse = Abbrev8Bit;<br>
+ if (Bits == SE_Char6)<br>
+ AbbrevToUse = Abbrev6Bit;<br>
+ else if (Bits == SE_Fixed7)<br>
+ AbbrevToUse = Abbrev7Bit;<br></blockquote><div><br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ Vals.push_back(MPSE.getValue().first);<br>
+<br>
+ for (const auto P : MPSE.getKey())<br>
+ Vals.push_back((unsigned char)P);<br>
+<br>
+ // Emit the finished record.<br>
+ Stream.EmitRecord(bitc::MST_CODE_ENTRY, Vals, AbbrevToUse);<br>
+<br>
+ Vals.clear();<br>
+ // Emit an optional hash for the module now<br>
+ auto &Hash = MPSE.getValue().second;<br>
+ bool AllZero =<br>
+ true; // Detect if the hash is empty, and do not generate it<br>
+ for (auto Val : Hash) {<br>
+ if (Val)<br>
+ AllZero = false;<br>
+ Vals.push_back(Val);<br>
+ }<br>
+ if (!AllZero) {<br>
+ // Emit the hash record.<br>
+ Stream.EmitRecord(bitc::MST_CODE_HASH, Vals, AbbrevHash);<br>
+ }<br>
<br>
- Vals.clear();<br>
- }<br>
+ Vals.clear();<br>
+ });<br>
Stream.ExitBlock();<br>
}<br>
<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></div></div>
</blockquote></div><br><br clear="all"><div><br></div></div><div class="gmail_extra">-- <br><div class="m_3069575451471421879gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> <a href="tel:(408)%20460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</div></blockquote></div></div>