[PATCH] D55234: Do not use a hash table to uniquify mergeable strings.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu created this revision.
ruiu added a reviewer: grimar.
Herald added subscribers: arichardson, aprantl, emaste.
Herald added a reviewer: espindola.

Previously, we have a hash table containing strings and their offsets to
manage mergeable strings. Technically we can live without that because we
can do binary search on a vector of mergeable strings to find a mergeable
strings. The table was there to speed up offset -> string piece lookup.

We recently observed that lld tend to consume more memory than gold when
linking executables with debug info, and we found that a few percent of
memory is consumed by the hash table. I wondered if we can save memory
here, so I run a few benchmarks with and without the hash table. Here is
the result.

  Speed (measured by `perf stat -r10`)
  
  Program         w/patch         w/o patch       Slowdown
  chrome          2.004718511     1.988568454     0.81%
  clang           0.518536707     0.503528155     2.98%
  clang-fsds      0.566316070     0.551120769     2.75%
  clang-gdb-index 5.091346130     5.052952745     0.75%
  gold            0.325922095     0.318968615     2.17%
  gold-fsds       0.353570003     0.342080243     3.35%
  linux-kernel    0.872819415     0.866891291     0.68%
  llvm-as         0.057709114     0.054467349     5.95%
  llvm-as-fsds    0.054338842     0.053751006     1.09%
  mozilla         3.730120566     3.836634236     -2.77%
  scylla          1.011386393     1.031144224     -1.91%
  
  Maximum RSS (measured by `time -v`)
  
  Program         w/patch         w/o patch       Memory saving
  chrome          1163088         1163572         0.04%
  clang           369364          373760          1.17%
  clang-fsds      392484          396712          1.06%
  clang-gdb-index 10391636        10391680        0.00%
  gold            227508          229108          0.69%
  gold-fsds       237308          238960          0.69%
  linux-kernel    143028          146932          2.65%
  llvm-as         46792           46976           0.39%
  llvm-as-fsds    47112           47336           0.47%
  mozilla         4631424         4833940         4.18%
  scylla          2712036         2793468         2.91%

It looks like the slowdown is not negligible, but lld got slower only when
the program being linked is small. For large programs, the regression
seems small, or it even got faster. Given that, I don't think having the
hash table is still a good tradeoff; we should drop the hash table to save
memory.


https://reviews.llvm.org/D55234

Files:
  lld/ELF/InputSection.cpp
  lld/ELF/InputSection.h


Index: lld/ELF/InputSection.h
===================================================================
--- lld/ELF/InputSection.h
+++ lld/ELF/InputSection.h
@@ -253,7 +253,6 @@
   // Splittable sections are handled as a sequence of data
   // rather than a single large blob of data.
   std::vector<SectionPiece> Pieces;
-  llvm::DenseMap<uint32_t, uint32_t> OffsetMap;
 
   // Returns I'th piece's data. This function is very hot when
   // string merging is enabled, so we want to inline.
Index: lld/ELF/InputSection.cpp
===================================================================
--- lld/ELF/InputSection.cpp
+++ lld/ELF/InputSection.cpp
@@ -1206,21 +1206,12 @@
     splitStrings(data(), Entsize);
   else
     splitNonStrings(data(), Entsize);
-
-  OffsetMap.reserve(Pieces.size());
-  for (size_t I = 0, E = Pieces.size(); I != E; ++I)
-    OffsetMap[Pieces[I].InputOff] = I;
 }
 
 SectionPiece *MergeInputSection::getSectionPiece(uint64_t Offset) {
   if (this->data().size() <= Offset)
     fatal(toString(this) + ": offset is outside the section");
 
-  // Find a piece starting at a given offset.
-  auto It = OffsetMap.find(Offset);
-  if (It != OffsetMap.end())
-    return &Pieces[It->second];
-
   // If Offset is not at beginning of a section piece, it is not in the map.
   // In that case we need to  do a binary search of the original section piece vector.
   size_t Size = Pieces.size();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55234.176474.patch
Type: text/x-patch
Size: 1409 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181203/64e95d3a/attachment.bin>