<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 26, 2015 at 2:47 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Mon, Oct 26, 2015 at 12:58 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: ruiu<br>
Date: Mon Oct 26 14:58:29 2015<br>
New Revision: 251337<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=251337&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=251337&view=rev</a><br>
Log:<br>
Optimize StringTableBuilder.<br>
<br>
This is a patch to improve StringTableBuilder's performance. That class'<br>
finalize function is very hot particularly in LLD because the function<br>
does tail-merge strings in string tables or SHF_MERGE sections.<br>
<br>
Generic std::sort-style sorter is not efficient for sorting strings.<br>
The function implemented in this patch seems to be more efficient.<br></blockquote><div><br></div></span><div><span style="font-size:13px">array_pod_sort uses a qsort-style comparator. Where is the peformance difference you are measuring coming from?</span></div></div></div></div></blockquote><div><br></div><div>The new algorithm does not compare common prefixes once we know that that are common. <a href="https://en.wikipedia.org/wiki/Multi-key_quicksort">https://en.wikipedia.org/wiki/Multi-key_quicksort</a></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">Also, did you measure using std::sort? Generally std::sort is generally faster due to inlining.</span></div></div></div></div></blockquote><div><br></div><div>I didn't.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">E.g. </span></div><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">std::sort(</span>&Strings[0], &Strings[0] + Strings.size(), [](StringPair LHS, StringPair RHS) {</div><div> auto rbegin = [](StringPair SP) { return std::reverse_iterator(SP.first.end()); };</div><div> auto rend = [](StringPair SP) { return std::reverse_iterator(SP.first.first()); };<br></div><div> return std::lexicographical_compare(rbegin(LHS), rend(LHS), rbegin(RHS), rend(RHS));</div><div>});</div><div><br></div><div>-- Sean Silva</div><div><div class="h5"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Here's a benchmark of LLD to link Clang with or without this patch.<br>
The numbers are medians of 50 runs.<br>
<br>
-O0<br>
real 0m0.455s<br>
real 0m0.430s (5.5% faster)<br>
<br>
-O3<br>
real 0m0.487s<br>
real 0m0.452s (7.2% faster)<br>
<br>
Since that is a benchmark of the whole linker, the speedup of<br>
StringTableBuilder itself is much more than that.<br>
<br>
<a href="http://reviews.llvm.org/D14053" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14053</a><br>
<br>
<br>
Modified:<br>
llvm/trunk/lib/MC/StringTableBuilder.cpp<br>
<br>
Modified: llvm/trunk/lib/MC/StringTableBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/StringTableBuilder.cpp?rev=251337&r1=251336&r2=251337&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/StringTableBuilder.cpp?rev=251337&r1=251336&r2=251337&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/MC/StringTableBuilder.cpp (original)<br>
+++ llvm/trunk/lib/MC/StringTableBuilder.cpp Mon Oct 26 14:58:29 2015<br>
@@ -18,20 +18,47 @@ using namespace llvm;<br>
<br>
StringTableBuilder::StringTableBuilder(Kind K) : K(K) {}<br>
<br>
-static int compareBySuffix(std::pair<StringRef, size_t> *const *AP,<br>
- std::pair<StringRef, size_t> *const *BP) {<br>
- StringRef A = (*AP)->first;<br>
- StringRef B = (*BP)->first;<br>
- size_t SizeA = A.size();<br>
- size_t SizeB = B.size();<br>
- size_t Len = std::min(SizeA, SizeB);<br>
- for (size_t I = 0; I < Len; ++I) {<br>
- char CA = A[SizeA - I - 1];<br>
- char CB = B[SizeB - I - 1];<br>
- if (CA != CB)<br>
- return CB - CA;<br>
+typedef std::pair<StringRef, size_t> StringPair;<br>
+<br>
+// Returns the character at Pos from end of a string.<br>
+static int charTailAt(StringPair *P, size_t Pos) {<br>
+ StringRef S = P->first;<br>
+ if (Pos >= S.size())<br>
+ return -1;<br>
+ return (unsigned char)S[S.size() - Pos - 1];<br>
+}<br>
+<br>
+// Three-way radix quicksort. This is much faster than std::sort with strcmp<br>
+// because it does not compare characters that we already know the same.<br>
+static void qsort(StringPair **Begin, StringPair **End, int Pos) {<br>
+tailcall:<br>
+ if (End - Begin <= 1)<br>
+ return;<br>
+<br>
+ // Partition items. Items in [Begin, P) are greater than the pivot,<br>
+ // [P, Q) are the same as the pivot, and [Q, End) are less than the pivot.<br>
+ int Pivot = charTailAt(*Begin, Pos);<br>
+ StringPair **P = Begin;<br>
+ StringPair **Q = End;<br>
+ for (StringPair **R = Begin + 1; R < Q;) {<br>
+ int C = charTailAt(*R, Pos);<br>
+ if (C > Pivot)<br>
+ std::swap(*P++, *R++);<br>
+ else if (C < Pivot)<br>
+ std::swap(*--Q, *R);<br>
+ else<br>
+ R++;<br>
+ }<br>
+<br>
+ qsort(Begin, P, Pos);<br>
+ qsort(Q, End, Pos);<br>
+ if (Pivot != -1) {<br>
+ // qsort(P, Q, Pos + 1), but with tail call optimization.<br>
+ Begin = P;<br>
+ End = Q;<br>
+ ++Pos;<br>
+ goto tailcall;<br>
}<br>
- return SizeB - SizeA;<br>
}<br>
<br>
void StringTableBuilder::finalize() {<br>
@@ -40,7 +67,8 @@ void StringTableBuilder::finalize() {<br>
for (std::pair<StringRef, size_t> &P : StringIndexMap)<br>
Strings.push_back(&P);<br>
<br>
- array_pod_sort(Strings.begin(), Strings.end(), compareBySuffix);<br>
+ if (!Strings.empty())<br>
+ qsort(&Strings[0], &Strings[0] + Strings.size(), 0);<br>
<br>
switch (K) {<br>
case RAW:<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><br></div></div>