[PDB] Save one type record copy

[rnk]

Summary

The TypeTableBuilder provides stable storage for type records. We don't
need to copy all of the bytes into a flat vector before adding it to the
TpiStreamBuilder.

This makes addTypeRecord take an ArrayRef<uint8_t> and a hash code to go
with it, which seems like a simplification.

[ruiu]

LGTM

lld/COFF/PDB.cpp

static ArrayRef<uint8_t> getDebugSection(ObjectFile *File, StringRef SecName) {
   if (Data.size() < 4)
     fatal(SecName + " too short");
   if (read32le(Data.data()) != COFF::DEBUG_SECTION_MAGIC)
     fatal(SecName + " has an invalid magic");
   return Data.slice(4);
 }
 
 static void addTypeInfo(pdb::TpiStreamBuilder &TpiBuilder,
-                        codeview::TypeTableBuilder &TypeTable,
-                        std::vector<uint8_t> &Data) {
+                        codeview::TypeTableBuilder &TypeTable) {
   // Start the TPI or IPI stream header.
   TpiBuilder.setVersionHeader(pdb::PdbTpiV80);
 
   // Flatten the in memory type table.
-  // FIXME: Avoid this copy.
   TypeTable.ForEachRecord([&](TypeIndex TI, ArrayRef<uint8_t> Rec) {
-    Data.insert(Data.end(), Rec.begin(), Rec.end());
+    // FIXME: Hash types.
+    TpiBuilder.addTypeRecord(Rec, None);
   });
-
-  BinaryByteStream Stream(Data, support::little);
-  codeview::CVTypeArray Records;
-  BinaryStreamReader Reader(Stream);
-  if (auto EC = Reader.readArray(Records, Reader.getLength()))
-    fatal(EC, "Reader.readArray failed");
-  for (const codeview::CVType &Rec : Records)
-    TpiBuilder.addTypeRecord(Rec);
 }
 
 // Merge .debug$T sections into IpiData and TpiData.
 static void mergeDebugT(SymbolTable *Symtab, pdb::PDBFileBuilder &Builder,
-                        std::vector<uint8_t> &TpiData,
-                        std::vector<uint8_t> &IpiData) {
+                        codeview::TypeTableBuilder &TypeTable,
+                        codeview::TypeTableBuilder &IDTable) {
   // Visit all .debug$T sections to add them to Builder.
-  codeview::TypeTableBuilder IDTable(BAlloc);
-  codeview::TypeTableBuilder TypeTable(BAlloc);
   for (ObjectFile *File : Symtab->ObjectFiles) {
     ArrayRef<uint8_t> Data = getDebugSection(File, ".debug$T");
     if (Data.empty())
       continue;
 
     BinaryByteStream Stream(Data, support::little);
     codeview::CVTypeArray Types;
     BinaryStreamReader Reader(Stream);
     // Follow type servers.  If the same type server is encountered more than
     // once for this instance of `PDBTypeServerHandler` (for example if many
     // object files reference the same TypeServer), the types from the
     // TypeServer will only be visited once.
     pdb::PDBTypeServerHandler Handler;
     Handler.addSearchPath(llvm::sys::path::parent_path(File->getName()));
     if (auto EC = Reader.readArray(Types, Reader.getLength()))
       fatal(EC, "Reader::readArray failed");
     if (auto Err =
             codeview::mergeTypeStreams(IDTable, TypeTable, &Handler, Types))
       fatal(Err, "codeview::mergeTypeStreams failed");
   }
 
   // Construct TPI stream contents.
-  addTypeInfo(Builder.getTpiBuilder(), TypeTable, TpiData);
+  addTypeInfo(Builder.getTpiBuilder(), TypeTable);
 
   // Construct IPI stream contents.
-  addTypeInfo(Builder.getIpiBuilder(), IDTable, IpiData);
+  addTypeInfo(Builder.getIpiBuilder(), IDTable);
 }
 
 static void dumpDebugT(ScopedPrinter &W, ObjectFile *File) {
   ListScope LS(W, "DebugT");
   ArrayRef<uint8_t> Data = getDebugSection(File, ".debug$T");
   if (Data.empty())
     return;
 
void coff::createPDB(StringRef Path, SymbolTable *Symtab,
   // Should be the current time, but set 0 for reproducibilty.
   InfoBuilder.setSignature(0);
   InfoBuilder.setVersion(pdb::PdbRaw_ImplVer::PdbImplVC70);
 
   // Add an empty DPI stream.
   auto &DbiBuilder = Builder.getDbiBuilder();
   DbiBuilder.setVersionHeader(pdb::PdbDbiV110);
 
-  std::vector<uint8_t> TpiData;
-  std::vector<uint8_t> IpiData;
-  mergeDebugT(Symtab, Builder, TpiData, IpiData);
+  codeview::TypeTableBuilder TypeTable(BAlloc);
+  codeview::TypeTableBuilder IDTable(BAlloc);
+  mergeDebugT(Symtab, Builder, TypeTable, IDTable);
 
   // Add Section Contributions.
   std::vector<pdb::SectionContrib> Contribs =
       pdb::DbiStreamBuilder::createSectionContribs(getInputSections(Symtab));
   DbiBuilder.setSectionContribs(Contribs);
 
   // Add Section Map stream.
   ArrayRef<object::coff_section> Sections = {

llvm/include/llvm/DebugInfo/PDB/Native/TpiStreamBuilder.h

 public:
   explicit TpiStreamBuilder(msf::MSFBuilder &Msf, uint32_t StreamIdx);
   ~TpiStreamBuilder();
 
   TpiStreamBuilder(const TpiStreamBuilder &) = delete;
   TpiStreamBuilder &operator=(const TpiStreamBuilder &) = delete;
 
   void setVersionHeader(PdbRaw_TpiVer Version);
-  void addTypeRecord(const codeview::CVType &Record);
+  void addTypeRecord(ArrayRef<uint8_t> Type, Optional<uint32_t> Hash);
 
   Error finalizeMsfLayout();
 
   Error commit(const msf::MSFLayout &Layout, WritableBinaryStreamRef Buffer);
 
   uint32_t calculateSerializedLength();
 
 private:
   uint32_t calculateHashBufferSize() const;
   Error finalize();
 
   msf::MSFBuilder &Msf;
   BumpPtrAllocator &Allocator;
 
+  size_t TypeRecordBytes = 0;
+
   Optional<PdbRaw_TpiVer> VerHeader;
-  std::vector<codeview::CVType> TypeRecords;
-  BinaryItemStream<codeview::CVType> TypeRecordStream;
+  std::vector<ArrayRef<uint8_t>> TypeRecords;
+  std::vector<uint32_t> TypeHashes;
   uint32_t HashStreamIndex = kInvalidStreamIndex;
   std::unique_ptr<BinaryByteStream> HashValueStream;
 
   const TpiStreamHeader *Header;
   uint32_t Idx;
 };
 }
 }
 
 #endif

llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp

 #include <cstdint>
 
 using namespace llvm;
 using namespace llvm::msf;
 using namespace llvm::pdb;
 using namespace llvm::support;
 
 TpiStreamBuilder::TpiStreamBuilder(MSFBuilder &Msf, uint32_t StreamIdx)
-    : Msf(Msf), Allocator(Msf.getAllocator()),
-      TypeRecordStream(llvm::support::little), Header(nullptr), Idx(StreamIdx) {
+    : Msf(Msf), Allocator(Msf.getAllocator()), Header(nullptr), Idx(StreamIdx) {
 }
 
 TpiStreamBuilder::~TpiStreamBuilder() = default;
 
 void TpiStreamBuilder::setVersionHeader(PdbRaw_TpiVer Version) {
   VerHeader = Version;
 }
 
-void TpiStreamBuilder::addTypeRecord(const codeview::CVType &Record) {
+void TpiStreamBuilder::addTypeRecord(ArrayRef<uint8_t> Record,
+                                     Optional<uint32_t> Hash) {
+  TypeRecordBytes += Record.size();
   TypeRecords.push_back(Record);
-  TypeRecordStream.setItems(TypeRecords);
+  if (Hash)
+    TypeHashes.push_back(*Hash);
 }
 
 Error TpiStreamBuilder::finalize() {
   if (Header)
     return Error::success();
 
   TpiStreamHeader *H = Allocator.Allocate<TpiStreamHeader>();
 
   uint32_t Count = TypeRecords.size();
   uint32_t HashBufferSize = calculateHashBufferSize();
 
   H->Version = *VerHeader;
   H->HeaderSize = sizeof(TpiStreamHeader);
   H->TypeIndexBegin = codeview::TypeIndex::FirstNonSimpleIndex;
   H->TypeIndexEnd = H->TypeIndexBegin + Count;
-  H->TypeRecordBytes = TypeRecordStream.getLength();
+  H->TypeRecordBytes = TypeRecordBytes;
 
   H->HashStreamIndex = HashStreamIndex;
   H->HashAuxStreamIndex = kInvalidStreamIndex;
   H->HashKeySize = sizeof(ulittle32_t);
   H->NumHashBuckets = MinTpiHashBuckets;
 
   // Recall that hash values go into a completely different stream identified by
   // the `HashStreamIndex` field of the `TpiStreamHeader`.  Therefore, the data
   // begins at offset 0 of this independent stream.
   H->HashValueBuffer.Off = 0;
   H->HashValueBuffer.Length = HashBufferSize;
   H->HashAdjBuffer.Off = H->HashValueBuffer.Off + H->HashValueBuffer.Length;
   H->HashAdjBuffer.Length = 0;
   H->IndexOffsetBuffer.Off = H->HashAdjBuffer.Off + H->HashAdjBuffer.Length;
   H->IndexOffsetBuffer.Length = 0;
 
   Header = H;
   return Error::success();
 }
 
 uint32_t TpiStreamBuilder::calculateSerializedLength() {
-  return sizeof(TpiStreamHeader) + TypeRecordStream.getLength();
+  return sizeof(TpiStreamHeader) + TypeRecordBytes;
 }
 
 uint32_t TpiStreamBuilder::calculateHashBufferSize() const {
-  if (TypeRecords.empty() || !TypeRecords[0].Hash.hasValue())
-    return 0;
-  return TypeRecords.size() * sizeof(ulittle32_t);
+  assert(TypeHashes.size() == TypeHashes.size() &&
+         "either all or no type records should have hashes");
+  return TypeHashes.size() * sizeof(ulittle32_t);
 }
 
 Error TpiStreamBuilder::finalizeMsfLayout() {
   uint32_t Length = calculateSerializedLength();
   if (auto EC = Msf.setStreamSize(Idx, Length))
     return EC;
 
   uint32_t HashBufferSize = calculateHashBufferSize();
 
   if (HashBufferSize == 0)
     return Error::success();
 
   auto ExpectedIndex = Msf.addStream(HashBufferSize);
   if (!ExpectedIndex)
     return ExpectedIndex.takeError();
   HashStreamIndex = *ExpectedIndex;
-  ulittle32_t *H = Allocator.Allocate<ulittle32_t>(TypeRecords.size());
-  MutableArrayRef<ulittle32_t> HashBuffer(H, TypeRecords.size());
-  for (uint32_t I = 0; I < TypeRecords.size(); ++I) {
-    HashBuffer[I] = *TypeRecords[I].Hash % MinTpiHashBuckets;
+  ulittle32_t *H = Allocator.Allocate<ulittle32_t>(TypeHashes.size());
+  MutableArrayRef<ulittle32_t> HashBuffer(H, TypeHashes.size());
+  for (uint32_t I = 0; I < TypeHashes.size(); ++I) {
+    HashBuffer[I] = TypeHashes[I] % MinTpiHashBuckets;
   }
   ArrayRef<uint8_t> Bytes(reinterpret_cast<const uint8_t *>(HashBuffer.data()),
                           HashBufferSize);
   HashValueStream =
       llvm::make_unique<BinaryByteStream>(Bytes, llvm::support::little);
   return Error::success();
 }
 
 Error TpiStreamBuilder::commit(const msf::MSFLayout &Layout,
                                WritableBinaryStreamRef Buffer) {
   if (auto EC = finalize())
     return EC;
 
   auto InfoS =
       WritableMappedBlockStream::createIndexedStream(Layout, Buffer, Idx);
 
   BinaryStreamWriter Writer(*InfoS);
   if (auto EC = Writer.writeObject(*Header))
     return EC;
 
-  auto RecordArray = VarStreamArray<codeview::CVType>(TypeRecordStream);
-  if (auto EC = Writer.writeArray(RecordArray))
+  for (auto Rec : TypeRecords)
+    if (auto EC = Writer.writeBytes(Rec))
       return EC;
 
   if (HashStreamIndex != kInvalidStreamIndex) {
     auto HVS = WritableMappedBlockStream::createIndexedStream(Layout, Buffer,
                                                               HashStreamIndex);
     BinaryStreamWriter HW(*HVS);
     if (auto EC = HW.writeStreamRef(*HashValueStream))
       return EC;
   }
 
   return Error::success();
 }

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

if (MI.Modi.hasValue()) {
         ModiBuilder.addSymbol(Symbol.Record);
     }
   }
 
   auto &TpiBuilder = Builder.getTpiBuilder();
   const auto &Tpi = YamlObj.TpiStream.getValueOr(DefaultTpiStream);
   TpiBuilder.setVersionHeader(Tpi.Version);
   for (const auto &R : Tpi.Records)
-    TpiBuilder.addTypeRecord(R.Record);
+    TpiBuilder.addTypeRecord(R.Record.data(), R.Record.Hash);
 
   const auto &Ipi = YamlObj.IpiStream.getValueOr(DefaultTpiStream);
   auto &IpiBuilder = Builder.getIpiBuilder();
   IpiBuilder.setVersionHeader(Ipi.Version);
   for (const auto &R : Ipi.Records)
-    IpiBuilder.addTypeRecord(R.Record);
+    TpiBuilder.addTypeRecord(R.Record.data(), R.Record.Hash);
 
   ExitOnErr(Builder.commit(opts::yaml2pdb::YamlPdbOutputFile));
 }
 
 static PDBFile &loadPDB(StringRef Path, std::unique_ptr<IPDBSession> &Session) {
   ExitOnErr(loadDataForPDB(PDB_ReaderType::Native, Path, Session));
 
   NativeSession *NS = static_cast<NativeSession *>(Session.get());