[PATCH] D36941: ms-pdb: Precompute deserialized symbols for pdb once and then use it in sort instead of doing it in the comparison function

[Alex Telishev via Phabricator via llvm-commits]

alex.telishev created this revision.
Herald added a subscriber: hiraditya.

`computeAddrMap` function calls `std::stable_sort` with a comparison function that computes deserialized symbols every time its called. In the result `deserializeAs<PublicSym32>` is called 20-30 times for every symbol. It's much faster to calculate it beforehand and pass pointer to it to the comparison function.


https://reviews.llvm.org/D36941

Files:
  llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp


Index: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
===================================================================
--- llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
+++ llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
@@ -169,34 +169,34 @@
   return Error::success();
 }
 
-static bool comparePubSymByAddrAndName(const CVSymbol *LS, const CVSymbol *RS) {
-  assert(LS->kind() == SymbolKind::S_PUB32);
-  assert(RS->kind() == SymbolKind::S_PUB32);
-
-  PublicSym32 PSL =
-      cantFail(SymbolDeserializer::deserializeAs<PublicSym32>(*LS));
-  PublicSym32 PSR =
-      cantFail(SymbolDeserializer::deserializeAs<PublicSym32>(*RS));
-
-  if (PSL.Segment != PSR.Segment)
-    return PSL.Segment < PSR.Segment;
-  if (PSL.Offset != PSR.Offset)
-    return PSL.Offset < PSR.Offset;
-
-  return PSL.Name < PSR.Name;
+static bool comparePubSymByAddrAndName(const std::pair<const CVSymbol *, const  PublicSym32 *> &LS, const std::pair<const CVSymbol *, const PublicSym32 *> &RS)
+{
+  if (LS.second->Segment != RS.second->Segment)
+    return LS.second->Segment < RS.second->Segment;
+  if (LS.second->Offset != RS.second->Offset)
+    return LS.second->Offset < RS.second->Offset;
+
+  return LS.second->Name < RS.second->Name;
 }
 
 /// Compute the address map. The address map is an array of symbol offsets
 /// sorted so that it can be binary searched by address.
 static std::vector<ulittle32_t> computeAddrMap(ArrayRef<CVSymbol> Records) {
   // Make a vector of pointers to the symbols so we can sort it by address.
   // Also gather the symbol offsets while we're at it.
-  std::vector<const CVSymbol *> PublicsByAddr;
+
+  std::vector<PublicSym32> DeserializedPublics;
+  std::vector<std::pair<const CVSymbol *, const PublicSym32 *>> PublicsByAddr;
   std::vector<uint32_t> SymOffsets;
+  DeserializedPublics.reserve (Records.size());
   PublicsByAddr.reserve(Records.size());
+  SymOffsets.reserve(Records.size());
+
   uint32_t SymOffset = 0;
   for (const CVSymbol &Sym : Records) {
-    PublicsByAddr.push_back(&Sym);
+    assert(Sym.kind() == SymbolKind::S_PUB32);
+    DeserializedPublics.push_back (cantFail (SymbolDeserializer::deserializeAs<PublicSym32>(Sym)));
+    PublicsByAddr.emplace_back (&Sym, &DeserializedPublics.back ());
     SymOffsets.push_back(SymOffset);
     SymOffset += Sym.length();
   }
@@ -206,8 +206,8 @@
   // Fill in the symbol offsets in the appropriate order.
   std::vector<ulittle32_t> AddrMap;
   AddrMap.reserve(Records.size());
-  for (const CVSymbol *Sym : PublicsByAddr) {
-    ptrdiff_t Idx = std::distance(Records.data(), Sym);
+  for (auto &Sym : PublicsByAddr) {
+    ptrdiff_t Idx = std::distance(Records.data(), Sym.first);
     assert(Idx >= 0 && size_t(Idx) < Records.size());
     AddrMap.push_back(ulittle32_t(SymOffsets[Idx]));
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36941.111904.patch
Type: text/x-patch
Size: 2805 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170820/8a124d6b/attachment.bin>