[llvm] r347687 - [PDB] Add symbol records in bulk

[Reid Kleckner via llvm-commits]

Author: rnk
Date: Tue Nov 27 11:00:23 2018
New Revision: 347687

URL: http://llvm.org/viewvc/llvm-project?rev=347687&view=rev
Log:
[PDB] Add symbol records in bulk

Summary:
This speeds up linking clang.exe/pdb with /DEBUG:GHASH by 31%, from
12.9s to 9.8s.

Symbol records are typically small (16.7 bytes on average), but we
processed them one at a time. CVSymbol is a relatively "large" type. It
wraps an ArrayRef<uint8_t> with a kind an optional 32-bit hash, which we
don't need. Before this change, each DbiModuleDescriptorBuilder would
maintain an array of CVSymbols, and would write them individually with a
BinaryItemStream.

With this change, we now add symbols that happen to appear contiguously
in bulk. For each .debug$S section (roughly one per function), we
allocate two copies, one for relocation, and one for realignment
purposes. For runs of symbols that go in the module stream, which is
most symbols, we now add them as a single ArrayRef<uint8_t>, so the
vector DbiModuleDescriptorBuilder is roughly linear in the number of
.debug$S sections (O(# funcs)) instead of the number of symbol records
(very large).

Some stats on symbol sizes for the curious:
  PDB size: 507M
  sym bytes: 316,508,016
  sym count:  18,954,971
  sym byte avg: 16.7

As future work, we may be able to skip copying symbol records in the
linker for realignment purposes if we make LLVM write them aligned into
the object file. We need to double check that such symbol records are
still compatible with link.exe, but if so, it's definitely worth doing,
since my profile shows we spend 500ms in memcpy in the symbol merging
code. We could potentially cut that in half by saving a copy.
Alternatively, we could apply the relocations *after* we iterate the
symbols. This would require some careful re-engineering of the
relocation processing code, though.

Reviewers: zturner, aganea, ruiu

Subscribers: hiraditya, llvm-commits

Differential Revision: https://reviews.llvm.org/D54554

Modified:
    llvm/trunk/include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h
    llvm/trunk/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp

Modified: llvm/trunk/include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h?rev=347687&r1=347686&r2=347687&view=diff
==============================================================================
--- llvm/trunk/include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h (original)
+++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h Tue Nov 27 11:00:23 2018
@@ -51,6 +51,7 @@ public:
   void setObjFileName(StringRef Name);
   void setFirstSectionContrib(const SectionContrib &SC);
   void addSymbol(codeview::CVSymbol Symbol);
+  void addSymbolsInBulk(ArrayRef<uint8_t> BulkSymbols);
 
   void
   addDebugSubsection(std::shared_ptr<codeview::DebugSubsection> Subsection);
@@ -91,7 +92,7 @@ private:
   std::string ModuleName;
   std::string ObjFileName;
   std::vector<std::string> SourceFiles;
-  std::vector<codeview::CVSymbol> Symbols;
+  std::vector<ArrayRef<uint8_t>> Symbols;
 
   std::vector<std::unique_ptr<codeview::DebugSubsectionRecordBuilder>>
       C13Builders;

Modified: llvm/trunk/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp?rev=347687&r1=347686&r2=347687&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp (original)
+++ llvm/trunk/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp Tue Nov 27 11:00:23 2018
@@ -19,7 +19,6 @@
 #include "llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h"
 #include "llvm/DebugInfo/PDB/Native/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
-#include "llvm/Support/BinaryItemStream.h"
 #include "llvm/Support/BinaryStreamWriter.h"
 
 using namespace llvm;
@@ -66,12 +65,22 @@ void DbiModuleDescriptorBuilder::setFirs
 }
 
 void DbiModuleDescriptorBuilder::addSymbol(CVSymbol Symbol) {
-  Symbols.push_back(Symbol);
-  // Symbols written to a PDB file are required to be 4 byte aligned.  The same
+  // Defer to the bulk API. It does the same thing.
+  addSymbolsInBulk(Symbol.data());
+}
+
+void DbiModuleDescriptorBuilder::addSymbolsInBulk(
+    ArrayRef<uint8_t> BulkSymbols) {
+  // Do nothing for empty runs of symbols.
+  if (BulkSymbols.empty())
+    return;
+
+  Symbols.push_back(BulkSymbols);
+  // Symbols written to a PDB file are required to be 4 byte aligned. The same
   // is not true of object files.
-  assert(Symbol.length() % alignOf(CodeViewContainer::Pdb) == 0 &&
+  assert(BulkSymbols.size() % alignOf(CodeViewContainer::Pdb) == 0 &&
          "Invalid Symbol alignment!");
-  SymbolByteSize += Symbol.length();
+  SymbolByteSize += BulkSymbols.size();
 }
 
 void DbiModuleDescriptorBuilder::addSourceFile(StringRef Path) {
@@ -145,16 +154,11 @@ Error DbiModuleDescriptorBuilder::commit
     if (auto EC =
             SymbolWriter.writeInteger<uint32_t>(COFF::DEBUG_SECTION_MAGIC))
       return EC;
-    BinaryItemStream<CVSymbol> Records(llvm::support::endianness::little);
-    Records.setItems(Symbols);
-    BinaryStreamRef RecordsRef(Records);
-    if (auto EC = SymbolWriter.writeStreamRef(RecordsRef))
-      return EC;
-    if (auto EC = SymbolWriter.padToAlignment(4))
-      return EC;
-    // TODO: Write C11 Line data
+    for (ArrayRef<uint8_t> Syms : Symbols)
+      SymbolWriter.writeBytes(Syms);
     assert(SymbolWriter.getOffset() % alignOf(CodeViewContainer::Pdb) == 0 &&
            "Invalid debug section alignment!");
+    // TODO: Write C11 Line data
     for (const auto &Builder : C13Builders) {
       assert(Builder && "Empty C13 Fragment Builder!");
       if (auto EC = Builder->commit(SymbolWriter))