<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Dec 9, 2016 at 8:30 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
So, my only comment is that this seems to be a bit too much effort to<br>
optimize string merging in a multi threaded environment. We should<br>
really look into what .dwo gets us and then see if there are still so<br>
many strings left to merge.<br></blockquote><div><br></div><div>Do you mean you want this patch to not be submitted? Split DWARF is one good thing, but I think this is also useful for a common use case.</div><div><br></div><div>Overall, this patch adds 170 lines and deletes 57 lines. If you subtract comment lines, this patch adds less than 100 lines. I don't think that's too complicated.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Cheers,<br>
Rafael<br>
<br>
<br>
Rui Ueyama via Phabricator via llvm-commits<br>
<div><div class="h5"><<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> writes:<br>
<br>
> ruiu created this revision.<br>
> ruiu added a reviewer: silvas.<br>
> ruiu added a subscriber: llvm-commits.<br>
><br>
> This is another attempt to speed up string merging. You want to read<br>
> the description of <a href="https://reviews.llvm.org/D27146" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D27146</a> first.<br>
><br>
> In this patch, I took a different approach than the probabilistic<br>
> algorithm used in <a href="https://reviews.llvm.org/D27146" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D27146</a>. Here is the algorithm.<br>
><br>
> The original code has a single hash table to merge strings. Now we<br>
> have N hash tables where N is a parallelism (currently N=16).<br>
><br>
> We invoke N threads. Each thread knows its thread index I where<br>
> 0 <= I < N. For each string S in a given string set, thread I adds S<br>
> to its own hash table only if hash(S) % N == I.<br>
><br>
> When all threads are done, there are N string tables with all<br>
> duplicated strings being merged.<br>
><br>
> There are pros and cons of this algorithm compared to the<br>
> probabilistic one.<br>
><br>
> Pros:<br>
><br>
> - It naturally produces deterministic output.<br>
> - Output is guaranteed to be the smallest.<br>
><br>
> Cons:<br>
><br>
> - Slower than the probabilistic algorithm due to the work it needs to do. N threads independently visit all strings, and because the number of mergeable strings it too large, even just skipping them is fairly expensive.<br>
><br>
>   On the other hand, the probabilistic algorithm doesn't need to skip any element.<br>
> - Unlike the probabilistic algorithm, it degrades performance if the number of available CPU core is smaller than N, because we now have more work to do in total than the original code.<br>
><br>
>   We can fix this if we are able to know in some way about how many cores are idle.<br>
><br>
> Here are perf results. The probabilistic algorithm completed the same<br>
> task in 5.227 seconds, so this algorithm is slower than that.<br>
><br>
>   Before:<br>
><br>
>      36095.759481 task-clock (msec)         #    5.539 CPUs utilized            ( +-  0.83% )<br>
>           191,033 context-switches          #    0.005 M/sec                    ( +-  0.22% )<br>
>             8,194 cpu-migrations            #    0.227 K/sec                    ( +- 12.24% )<br>
>         2,342,017 page-faults               #    0.065 M/sec                    ( +-  0.06% )<br>
>    99,758,779,851 cycles                    #    2.764 GHz                      ( +-  0.79% )<br>
>    80,526,137,412 stalled-cycles-frontend   #   80.72% frontend cycles idle     ( +-  0.95% )<br>
>   <not supported> stalled-cycles-backend<br>
>    46,308,518,501 instructions              #    0.46  insns per cycle<br>
>                                             #    1.74  stalled cycles per insn  ( +-  0.12% )<br>
>     8,962,860,074 branches                  #  248.308 M/sec                    ( +-  0.17% )<br>
>       149,264,611 branch-misses             #    1.67% of all branches          ( +-  0.06% )<br>
><br>
>       6.517101649 seconds time elapsed                                          ( +-  0.42% )<br>
><br>
>   After:<br>
><br>
>      45346.098328 task-clock (msec)         #    8.002 CPUs utilized            ( +-  0.77% )<br>
>           165,487 context-switches          #    0.004 M/sec                    ( +-  0.24% )<br>
>             7,455 cpu-migrations            #    0.164 K/sec                    ( +- 11.13% )<br>
>         2,347,870 page-faults               #    0.052 M/sec                    ( +-  0.84% )<br>
>   125,725,992,168 cycles                    #    2.773 GHz                      ( +-  0.76% )<br>
>    96,550,047,016 stalled-cycles-frontend   #   76.79% frontend cycles idle     ( +-  0.89% )<br>
>   <not supported> stalled-cycles-backend<br>
>    79,847,589,597 instructions              #    0.64  insns per cycle<br>
>                                             #    1.21  stalled cycles per insn  ( +-  0.22% )<br>
>    13,569,202,477 branches                  #  299.236 M/sec                    ( +-  0.28% )<br>
>       200,343,507 branch-misses             #    1.48% of all branches          ( +-  0.16% )<br>
><br>
>       5.666585908 seconds time elapsed                                          ( +-  0.67% )<br>
><br>
> To conclude, I lean towards the probabilistic algorithm if we can<br>
> make its output deterministic, since its faster in any sitaution<br>
> (except for pathetic inputs in which our assumption that most<br>
> duplicated strings are spread across inputs doesn't hold.)<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D27152" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D27152</a><br>
><br>
> Files:<br>
>   ELF/InputSection.h<br>
>   ELF/OutputSections.cpp<br>
>   ELF/OutputSections.h<br>
><br>
</div></div>> Index: ELF/OutputSections.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- ELF/OutputSections.h<br>
> +++ ELF/OutputSections.h<br>
> @@ -16,12 +16,14 @@<br>
>  #include "lld/Core/LLVM.h"<br>
>  #include "llvm/MC/StringTableBuilder.h"<br>
>  #include "llvm/Object/ELF.h"<br>
> +#include <functional><br>
><br>
>  namespace lld {<br>
>  namespace elf {<br>
><br>
>  class SymbolBody;<br>
>  struct EhSectionPiece;<br>
> +struct SectionPiece;<br>
>  template <class ELFT> class EhInputSection;<br>
>  template <class ELFT> class InputSection;<br>
>  template <class ELFT> class InputSectionBase;<br>
> @@ -142,9 +144,12 @@<br>
>  private:<br>
>    void finalizeTailMerge();<br>
>    void finalizeNoTailMerge();<br>
> +  void forEachPiece(<br>
> +      std::function<void(<wbr>SectionPiece &Piece, llvm::CachedHashStringRef S)> Fn);<br>
><br>
>    llvm::StringTableBuilder Builder;<br>
>    std::vector<MergeInputSection<<wbr>ELFT> *> Sections;<br>
> +  size_t StringAlignment;<br>
>  };<br>
><br>
>  struct CieRecord {<br>
> Index: ELF/OutputSections.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- ELF/OutputSections.cpp<br>
> +++ ELF/OutputSections.cpp<br>
> @@ -21,6 +21,7 @@<br>
>  #include "llvm/Support/MD5.h"<br>
>  #include "llvm/Support/MathExtras.h"<br>
>  #include "llvm/Support/SHA1.h"<br>
> +#include <atomic><br>
><br>
>  using namespace llvm;<br>
>  using namespace llvm::dwarf;<br>
> @@ -470,10 +471,21 @@<br>
>  MergeOutputSection<ELFT>::<wbr>MergeOutputSection(StringRef Name, uint32_t Type,<br>
>                                               uintX_t Flags, uintX_t Alignment)<br>
>      : OutputSectionBase(Name, Type, Flags),<br>
> -      Builder(StringTableBuilder::<wbr>RAW, Alignment) {}<br>
> +      Builder(StringTableBuilder::<wbr>RAW, Alignment), StringAlignment(Alignment) {<br>
> +  assert(Alignment != 0 && isPowerOf2_64(Alignment));<br>
> +}<br>
><br>
>  template <class ELFT> void MergeOutputSection<ELFT>::<wbr>writeTo(uint8_t *Buf) {<br>
> -  Builder.write(Buf);<br>
> +  if (shouldTailMerge()) {<br>
> +    Builder.write(Buf);<br>
> +    return;<br>
> +  }<br>
> +<br>
> +  // Builder is not used for sharded string table construction.<br>
> +  forEachPiece([&](SectionPiece &Piece, CachedHashStringRef S) {<br>
> +    if (Piece.First)<br>
> +      memcpy(Buf + Piece.OutputOff, S.val().data(), S.size());<br>
> +  });<br>
>  }<br>
><br>
>  template <class ELFT><br>
> @@ -524,11 +536,78 @@<br>
<span class="">>    this->Size = Builder.getSize();<br>
>  }<br>
><br>
> +static size_t align2(size_t Val, size_t Alignment) {<br>
> +  return (Val + Alignment - 1) & ~(Alignment - 1);<br>
</span>> +}<br>
> +<br>
> +// Call Fn for each section piece.<br>
> +template <class ELFT><br>
> +void MergeOutputSection<ELFT>::<wbr>forEachPiece(<br>
> +    std::function<void(<wbr>SectionPiece &Piece, CachedHashStringRef S)> Fn) {<br>
> +  for (MergeInputSection<ELFT> *Sec : Sections)<br>
> +    for (size_t I = 0, E = Sec->Pieces.size(); I != E; ++I)<br>
> +      if (Sec->Pieces[I].Live)<br>
> +        Fn(Sec->Pieces[I], Sec->getData(I));<br>
> +}<br>
> +<br>
> +// Split a vector Vec into smaller vectors.<br>
> +template <class T><br>
> +static std::vector<std::vector<T>> split(std::vector<T> Vec, size_t NumShards) {<br>
> +  std::vector<std::vector<T>> Ret(NumShards);<br>
> +  size_t I = 0;<br>
> +  for (T &Elem : Vec)<br>
> +    Ret[I++ % NumShards].push_back(Elem);<br>
> +  return Ret;<br>
> +}<br>
> +<br>
>  template <class ELFT> void MergeOutputSection<ELFT>::<wbr>finalize() {<br>
> -  if (shouldTailMerge())<br>
> +  if (shouldTailMerge()) {<br>
>      finalizeTailMerge();<br>
> -  else<br>
> -    finalizeNoTailMerge();<br>
> +    return;<br>
> +  }<br>
> +<br>
> +  const int NumShards = 16;<br>
<span class="">> +  DenseMap<CachedHashStringRef, size_t> OffsetMap[NumShards];<br>
> +  size_t ShardSize[NumShards];<br>
</span>> +<br>
> +  // Construct NumShards number of string tables in parallel.<br>
> +  parallel_for(0, NumShards, [&](int Idx) {<br>
> +    size_t Offset = 0;<br>
> +    forEachPiece([&](SectionPiece &Piece, CachedHashStringRef S) {<br>
> +      if (S.hash() % NumShards != Idx)<br>
> +        return;<br>
> +<br>
> +      size_t Off = align2(Offset, StringAlignment);<br>
> +      auto P = OffsetMap[Idx].insert({S, Off});<br>
<span class="">> +      if (P.second) {<br>
> +        Piece.First = true;<br>
> +        Piece.OutputOff = Off;<br>
</span>> +        Offset = Off + S.size();<br>
> +      } else {<br>
> +        Piece.OutputOff = P.first->second;<br>
> +      }<br>
> +    });<br>
> +    ShardSize[Idx] = Offset;<br>
> +  });<br>
> +<br>
> +  // Piece.OutputOff was set independently, so we need to fix it.<br>
> +  // First, we compute starting offset in the string table for each shard.<br>
> +  size_t ShardOffset[NumShards];<br>
> +  ShardOffset[0] = 0;<br>
> +  for (int I = 1; I != NumShards; ++I)<br>
> +    ShardOffset[I] = ShardOffset[I - 1] + ShardSize[I - 1];<br>
> +<br>
<span class="">> +  // Add a shard starting offset to each section piece.<br>
> +  parallel_for_each(Sections.<wbr>begin(), Sections.end(),<br>
> +                    [&](MergeInputSection<ELFT> *Sec) {<br>
</span>> +                      for (size_t I = 0, E = Sec->Pieces.size(); I != E; ++I)<br>
> +                        if (Sec->Pieces[I].Live)<br>
> +                          Sec->Pieces[I].OutputOff +=<br>
> +                              ShardOffset[Sec->getData(I).<wbr>hash() % NumShards];<br>
> +                    });<br>
> +<br>
> +  // Set the size of this output section.<br>
> +  this->Size = ShardOffset[NumShards - 1] + ShardSize[NumShards - 1];<br>
>  }<br>
><br>
>  template <class ELFT><br>
> Index: ELF/InputSection.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- ELF/InputSection.h<br>
> +++ ELF/InputSection.h<br>
> @@ -160,11 +160,13 @@<br>
>  // be found by looking at the next one) and put the hash in a side table.<br>
>  struct SectionPiece {<br>
>    SectionPiece(size_t Off, bool Live = false)<br>
> -      : InputOff(Off), OutputOff(-1), Live(Live || !Config->GcSections) {}<br>
> +      : InputOff(Off), Live(Live || !Config->GcSections), OutputOff(-1),<br>
> +        First(false) {}<br>
><br>
> -  size_t InputOff;<br>
> -  ssize_t OutputOff : 8 * sizeof(ssize_t) - 1;<br>
> +  size_t InputOff : 8 * sizeof(size_t) - 1;<br>
>    size_t Live : 1;<br>
> +  ssize_t OutputOff : 8 * sizeof(ssize_t) - 1;<br>
> +  size_t First : 1;<br>
>  };<br>
>  static_assert(sizeof(<wbr>SectionPiece) == 2 * sizeof(size_t),<br>
>                "SectionPiece is too big");<br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">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/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>