From 1bc2ce6b9b3dc854e06630f3a6bec83876b3061c Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Thu, 18 Jan 2018 18:35:01 +0000 Subject: [PATCH] Speed up iteration of CodeView record streams. There's some abstraction overhead in the underlying mechanisms that were being used, and it was leading to an abundance of small but not-free copies being made. This showed up on a profile. Eliminating this and going back to a low-level byte-based implementation speeds up lld with /DEBUG between 10 and 15%. Differential Revision: https://reviews.llvm.org/D42148 llvm-svn: 322871 --- lld/COFF/PDB.cpp | 85 ++++++++++--------- .../llvm/DebugInfo/CodeView/CVRecord.h | 24 ++++++ .../DebugInfo/CodeView/TypeStreamMerger.cpp | 10 ++- 3 files changed, 76 insertions(+), 43 deletions(-) diff --git a/lld/COFF/PDB.cpp b/lld/COFF/PDB.cpp index 7c620e484a91..e8395c844814 100644 --- a/lld/COFF/PDB.cpp +++ b/lld/COFF/PDB.cpp @@ -677,54 +677,61 @@ static void mergeSymbolRecords(BumpPtrAllocator &Alloc, ObjFile *File, BinaryStreamRef SymData) { // FIXME: Improve error recovery by warning and skipping records when // possible. - CVSymbolArray Syms; - BinaryStreamReader Reader(SymData); - ExitOnErr(Reader.readArray(Syms, Reader.getLength())); + ArrayRef SymsBuffer; + cantFail(SymData.readBytes(0, SymData.getLength(), SymsBuffer)); SmallVector Scopes; - for (CVSymbol Sym : Syms) { - // Discover type index references in the record. Skip it if we don't know - // where they are. - SmallVector TypeRefs; - if (!discoverTypeIndicesInSymbol(Sym, TypeRefs)) { - log("ignoring unknown symbol record with kind 0x" + utohexstr(Sym.kind())); - continue; - } - // Copy the symbol record so we can mutate it. - MutableArrayRef NewData = copySymbolForPdb(Sym, Alloc); + auto EC = forEachCodeViewRecord( + SymsBuffer, [&](const CVSymbol &Sym) -> llvm::Error { + // Discover type index references in the record. Skip it if we don't + // know where they are. + SmallVector TypeRefs; + if (!discoverTypeIndicesInSymbol(Sym, TypeRefs)) { + log("ignoring unknown symbol record with kind 0x" + + utohexstr(Sym.kind())); + return Error::success(); + } - // Re-map all the type index references. - MutableArrayRef Contents = - NewData.drop_front(sizeof(RecordPrefix)); - remapTypesInSymbolRecord(File, Sym.kind(), Contents, IndexMap, TypeRefs); + // Copy the symbol record so we can mutate it. + MutableArrayRef NewData = copySymbolForPdb(Sym, Alloc); - // An object file may have S_xxx_ID symbols, but these get converted to - // "real" symbols in a PDB. - translateIdSymbols(NewData, IDTable); + // Re-map all the type index references. + MutableArrayRef Contents = + NewData.drop_front(sizeof(RecordPrefix)); + remapTypesInSymbolRecord(File, Sym.kind(), Contents, IndexMap, + TypeRefs); - // If this record refers to an offset in the object file's string table, - // add that item to the global PDB string table and re-write the index. - recordStringTableReferences(Sym.kind(), Contents, StringTableRefs); + // An object file may have S_xxx_ID symbols, but these get converted to + // "real" symbols in a PDB. + translateIdSymbols(NewData, IDTable); - SymbolKind NewKind = symbolKind(NewData); + // If this record refers to an offset in the object file's string table, + // add that item to the global PDB string table and re-write the index. + recordStringTableReferences(Sym.kind(), Contents, StringTableRefs); - // Fill in "Parent" and "End" fields by maintaining a stack of scopes. - CVSymbol NewSym(NewKind, NewData); - if (symbolOpensScope(NewKind)) - scopeStackOpen(Scopes, File->ModuleDBI->getNextSymbolOffset(), NewSym); - else if (symbolEndsScope(NewKind)) - scopeStackClose(Scopes, File->ModuleDBI->getNextSymbolOffset(), File); + SymbolKind NewKind = symbolKind(NewData); - // Add the symbol to the globals stream if necessary. Do this before adding - // the symbol to the module since we may need to get the next symbol offset, - // and writing to the module's symbol stream will update that offset. - if (symbolGoesInGlobalsStream(NewSym)) - addGlobalSymbol(GsiBuilder, *File, NewSym); + // Fill in "Parent" and "End" fields by maintaining a stack of scopes. + CVSymbol NewSym(NewKind, NewData); + if (symbolOpensScope(NewKind)) + scopeStackOpen(Scopes, File->ModuleDBI->getNextSymbolOffset(), + NewSym); + else if (symbolEndsScope(NewKind)) + scopeStackClose(Scopes, File->ModuleDBI->getNextSymbolOffset(), File); - // Add the symbol to the module. - if (symbolGoesInModuleStream(NewSym)) - File->ModuleDBI->addSymbol(NewSym); - } + // Add the symbol to the globals stream if necessary. Do this before + // adding the symbol to the module since we may need to get the next + // symbol offset, and writing to the module's symbol stream will update + // that offset. + if (symbolGoesInGlobalsStream(NewSym)) + addGlobalSymbol(GsiBuilder, *File, NewSym); + + // Add the symbol to the module. + if (symbolGoesInModuleStream(NewSym)) + File->ModuleDBI->addSymbol(NewSym); + return Error::success(); + }); + cantFail(std::move(EC)); } // Allocate memory for a .debug$S section and relocate it. diff --git a/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h b/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h index 9f3a753ad1ae..596996d94519 100644 --- a/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h +++ b/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h @@ -61,6 +61,30 @@ template struct RemappedRecord { SmallVector, 8> Mappings; }; +template +Error forEachCodeViewRecord(ArrayRef StreamBuffer, Func F) { + while (!StreamBuffer.empty()) { + if (StreamBuffer.size() < sizeof(RecordPrefix)) + return make_error(cv_error_code::corrupt_record); + + const RecordPrefix *Prefix = + reinterpret_cast(StreamBuffer.data()); + + uint16_t RealLen = Prefix->RecordLen + 2; + if (StreamBuffer.size() < RealLen) + return make_error(cv_error_code::corrupt_record); + + ArrayRef Data = StreamBuffer.take_front(RealLen); + StreamBuffer = StreamBuffer.drop_front(RealLen); + + Record R(static_cast((uint16_t)Prefix->RecordKind), + Data); + if (auto EC = F(R)) + return EC; + } + return Error::success(); +} + /// Read a complete record from a stream at a random offset. template inline Expected> readCVRecordFromStream(BinaryStreamRef Stream, diff --git a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp index 6a94952c175b..f1ebd23c563f 100644 --- a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp +++ b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp @@ -346,10 +346,12 @@ Error TypeStreamMerger::doit(const CVTypeArray &Types) { } Error TypeStreamMerger::remapAllTypes(const CVTypeArray &Types) { - for (const CVType &Type : Types) - if (auto EC = remapType(Type)) - return EC; - return Error::success(); + BinaryStreamRef Stream = Types.getUnderlyingStream(); + ArrayRef Buffer; + cantFail(Stream.readBytes(0, Stream.getLength(), Buffer)); + + return forEachCodeViewRecord( + Buffer, [this](const CVType &T) { return remapType(T); }); } Error TypeStreamMerger::remapType(const CVType &Type) {