[PATCH] D94555: [LLD][COFF] Avoid std::vector resizes during type merging

[Alexandre Ganea via Phabricator via llvm-commits]

aganea created this revision.
aganea requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

As discussed in https://reviews.llvm.org/D87805#2491679 the `MergedInfo::recs` vector can get quite large and resizing it while remapping types creates unnecessary copies and virtual memory page faults.

This patch saves consistently approx. 0.6 sec (out of 18 sec).

With 12 hyper-threads:

  Benchmark #1: before\lld-link.exe @link.rsp /threads:12
    Time (mean ± σ):     17.939 s ±  1.215 s    [User: 2.7 ms, System: 3.5 ms]
    Range (min … max):   15.537 s … 18.597 s    10 runs
  
  Benchmark #2: after\lld-link.exe @link.rsp /threads:12
    Time (mean ± σ):     17.298 s ±  1.511 s    [User: 1.4 ms, System: 8.9 ms]
    Range (min … max):   15.512 s … 18.513 s    10 runs

With 36 hyper-threads (thus using only one CPU socket):

  Benchmark #1: before\lld-link.exe @link.rsp
    Time (mean ± σ):     18.085 s ±  0.764 s    [User: 2.7 ms, System: 3.3 ms]
    Range (min … max):   15.918 s … 18.444 s    10 runs
  
  Benchmark #2: after\lld-link.exe @link.rsp
    Time (mean ± σ):     17.453 s ±  1.147 s    [User: 2.7 ms, System: 8.7 ms]
    Range (min … max):   15.766 s … 18.246 s    10 runs

Summary

  'f:\aganea\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @link.rsp' ran
    1.04 ± 0.08 times faster than 'f:\aganea\llvm-project\buildninjaRelRpMalloc2\bin\old\lld-link.exe @link.rsp'

F:\aganea\anvilp\games\ack\projects\win64>hyperfine --show-output "f:\aganea\llvm-project\buildninjaRelRpMalloc2\bin\old\lld-link.exe @link.rsp /threads:36" "f:\aganea\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @link.rsp /threads:36"
Benchmark #1: f:\aganea\llvm-project\buildninjaRelRpMalloc2\bin\old\lld-link.exe @link.rsp /threads:36

  Time (mean ± σ):     17.787 s ±  0.747 s    [User: 4.2 ms, System: 5.6 ms]
  Range (min … max):   15.666 s … 18.059 s    10 runs

Benchmark #2: f:\aganea\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @link.rsp /threads:36

  Time (mean ± σ):     17.102 s ±  1.323 s    [User: 2.6 ms, System: 4.0 ms]
  Range (min … max):   15.175 s … 18.023 s    10 runs


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94555

Files:
  lld/COFF/DebugTypes.cpp
  lld/COFF/DebugTypes.h


Index: lld/COFF/DebugTypes.h
===================================================================
--- lld/COFF/DebugTypes.h
+++ lld/COFF/DebugTypes.h
@@ -72,6 +72,9 @@
   void remapRecord(MutableArrayRef<uint8_t> rec,
                    ArrayRef<llvm::codeview::TiReference> typeRefs);
 
+  void computeTypeRecordSize(llvm::codeview::CVType ty, unsigned &nbTpiRecs,
+                             unsigned &nbIpiRecs);
+
   void mergeTypeRecord(TypeIndex curIndex, llvm::codeview::CVType ty);
 
   // Merge the type records listed in uniqueTypes. beginIndex is the TypeIndex
Index: lld/COFF/DebugTypes.cpp
===================================================================
--- lld/COFF/DebugTypes.cpp
+++ lld/COFF/DebugTypes.cpp
@@ -620,6 +620,17 @@
   });
 }
 
+void TpiSource::computeTypeRecordSize(CVType ty, unsigned &nbTpiRecs,
+                                      unsigned &nbIpiRecs) {
+  assert(ty.length() <= codeview::MaxRecordLength);
+  size_t newSize = alignTo(ty.length(), 4);
+
+  // Decide if the merged type goes into TPI or IPI.
+  bool isItem = isIdRecord(ty.kind());
+
+  (isItem ? nbIpiRecs : nbTpiRecs) += newSize;
+}
+
 void TpiSource::mergeTypeRecord(TypeIndex curIndex, CVType ty) {
   // Decide if the merged type goes into TPI or IPI.
   bool isItem = isIdRecord(ty.kind());
@@ -679,6 +690,24 @@
   auto nextUniqueIndex = uniqueTypes.begin();
   assert(mergedTpi.recs.empty());
   assert(mergedIpi.recs.empty());
+
+  // Pre-compute the number of elements in advance to avoid std::vector resizes.
+  unsigned nbTpiRecs = 0;
+  unsigned nbIpiRecs = 0;
+  forEachTypeChecked(typeRecords, [&](const CVType &ty) {
+    if (nextUniqueIndex != uniqueTypes.end() &&
+        *nextUniqueIndex == ghashIndex) {
+      computeTypeRecordSize(ty, nbTpiRecs, nbIpiRecs);
+      ++nextUniqueIndex;
+    }
+    ++ghashIndex;
+  });
+  mergedTpi.recs.reserve(nbTpiRecs);
+  mergedIpi.recs.reserve(nbIpiRecs);
+
+  // Do the actual type merge.
+  ghashIndex = 0;
+  nextUniqueIndex = uniqueTypes.begin();
   forEachTypeChecked(typeRecords, [&](const CVType &ty) {
     if (nextUniqueIndex != uniqueTypes.end() &&
         *nextUniqueIndex == ghashIndex) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94555.316241.patch
Type: text/x-patch
Size: 2168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210112/f1907bb4/attachment-0001.bin>