[PGO] Improve Indexed Profile Reader efficiency

With the support of value profiling added, the Indexed prof
reader gets less efficient. The prof reader initialization
used to be just reading the file header, but with VP support
added, initialization needs to walk through all profile keys
of ondisk hash table resulting in very poor locality and large
memory increase (keys are stored together with the profile data
in the mapped profile buffer). Even worse, when the reader is 
used by the compiler (not llvm-profdata too), the penalty becomes
very high as compilation of each single module requires touching
profile data buffer for the whole program. 

In this patch, the icall target values (MD5hash) are no longer eargerly 
converted back to name strings when the data is read into memory. New
interface is added to to profile reader so that InstrProfSymtab can be
lazily created for Indexed profile reader on-demand. Creating of the 
symtab is intended to be used by llvm-profdata tool for symbolic dumping
of  VP data. It can be used with compiler (for legacy out of tree uses)
too but not recommended due to compile time and memory reasons 
mentioned above.

Some other cleanups are also included: Function Addr to md5 map is now
consolated into InstrProfSymtab. InstrProfStringtab is no longer used and
eliminated.

llvm-svn: 256114

include/llvm/ProfileData/InstrProf.h

@@ -213,78 +213,89 @@ uint64_t ComputeHash(StringRef K);
 /// data of the function. See \c getPGOFuncName() method for details
 /// on how PGO name is formed.
 class InstrProfSymtab {
+public:
+  typedef std::vector<std::pair<uint64_t, uint64_t>> AddrHashMap;
+
 private:
   StringRef Data;
   uint64_t Address;
+  // A map from MD5 hash keys to function name strings.
   std::vector<std::pair<uint64_t, std::string>> HashNameMap;
+  // A map from function runtime address to function name MD5 hash.
+  // This map is only populated and used by raw instr profile reader.
+  AddrHashMap AddrToMD5Map;
 
 public:
-  InstrProfSymtab() : Data(), Address(0) {}
+  InstrProfSymtab() : Data(), Address(0), HashNameMap(), AddrToMD5Map() {}
 
-  /// Create InstrProfSymtab from a object file section which
+  /// Create InstrProfSymtab from an object file section which
   /// contains function PGO names that are uncompressed.
+  /// This interface is used by CoverageMappingReader.
   std::error_code create(object::SectionRef &Section);
-  std::error_code create(StringRef D, uint64_t BaseAddr) {
+  /// This interface is used by reader of CoverageMapping test
-    Data = D;
+  /// format.
-    Address = BaseAddr;
+  inline std::error_code create(StringRef D, uint64_t BaseAddr);
-    return std::error_code();
+  /// Create InstrProfSymtab from a set of names iteratable from
-  }
+  /// \p IterRange. This interface is used by IndexedProfReader.
-  template <typename NameIterRange> void create(NameIterRange &IterRange) {
+  template <typename NameIterRange> void create(const NameIterRange &IterRange);
-    for (auto Name : IterRange)
+  // If the symtab is created by a series of calls to \c addFuncName, \c
-      HashNameMap.push_back(
+  // finalizeSymtab needs to be called before looking up function names.
-          std::make_pair(IndexedInstrProf::ComputeHash(Name), Name.str()));
+  // This is required because the underlying map is a vector (for space
-    finalizeSymtab();
+  // efficiency) which needs to be sorted.
-  }
+  inline void finalizeSymtab();
-
+  /// Update the symtab by adding \p FuncName to the table. This interface
-  // If the symtab is created by a series calls to \c addFuncName, \c
+  /// is used by the raw and text profile readers.
-  // finalizeSymtab needs to
-  // be called before function name/symbol lookup using MD5 hash. This is
-  // required because
-  // the underlying map is vector (for space efficiency) which needs to be
-  // sorted.
-  void finalizeSymtab() {
-    std::sort(HashNameMap.begin(), HashNameMap.end(), less_first());
-    HashNameMap.erase(std::unique(HashNameMap.begin(), HashNameMap.end()),
-                      HashNameMap.end());
-  }
-
   void addFuncName(StringRef FuncName) {
     HashNameMap.push_back(std::make_pair(
         IndexedInstrProf::ComputeHash(FuncName), FuncName.str()));
   }
-
+  /// Map a function address to its name's MD5 hash. This interface
+  /// is only used by the raw profiler reader.
+  void mapAddress(uint64_t Addr, uint64_t MD5Val) {
+    AddrToMD5Map.push_back(std::make_pair(Addr, MD5Val));
+  }
+  AddrHashMap &getAddrHashMap() { return AddrToMD5Map; }
   /// Return function's PGO name from the function name's symabol
   /// address in the object file. If an error occurs, Return
   /// an empty string.
   StringRef getFuncName(uint64_t FuncNameAddress, size_t NameSize);
   /// Return function's PGO name from the name's md5 hash value.
   /// If not found, return an empty string.
-  StringRef getFuncName(uint64_t FuncMD5Hash) {
+  inline StringRef getFuncName(uint64_t FuncMD5Hash);
-    auto Result =
-        std::lower_bound(HashNameMap.begin(), HashNameMap.end(), FuncMD5Hash,
-                         [](const std::pair<uint64_t, std::string> &LHS,
-                            uint64_t RHS) { return LHS.first < RHS; });
-    if (Result != HashNameMap.end())
-      return Result->second;
-    return StringRef();
-  }
 };
 
-struct InstrProfStringTable {
+std::error_code InstrProfSymtab::create(StringRef D, uint64_t BaseAddr) {
-  // Set of string values in profiling data.
+  Data = D;
-  StringSet<> StringValueSet;
+  Address = BaseAddr;
-  InstrProfStringTable() { StringValueSet.clear(); }
+  return std::error_code();
-  // Get a pointer to internal storage of a string in set
+}
-  const char *getStringData(StringRef Str) {
+
-    auto Result = StringValueSet.find(Str);
+template <typename NameIterRange>
-    return (Result == StringValueSet.end()) ? nullptr : Result->first().data();
+void InstrProfSymtab::create(const NameIterRange &IterRange) {
-  }
+  for (auto Name : IterRange)
-  // Insert a string to StringTable
+    HashNameMap.push_back(
-  const char *insertString(StringRef Str) {
+        std::make_pair(IndexedInstrProf::ComputeHash(Name), Name.str()));
-    auto Result = StringValueSet.insert(Str);
+  finalizeSymtab();
-    return Result.first->first().data();
+}
-  }
+
-};
+void InstrProfSymtab::finalizeSymtab() {
+  std::sort(HashNameMap.begin(), HashNameMap.end(), less_first());
+  HashNameMap.erase(std::unique(HashNameMap.begin(), HashNameMap.end()),
+                    HashNameMap.end());
+  std::sort(AddrToMD5Map.begin(), AddrToMD5Map.end(), less_first());
+  AddrToMD5Map.erase(std::unique(AddrToMD5Map.begin(), AddrToMD5Map.end()),
+                     AddrToMD5Map.end());
+}
+
+StringRef InstrProfSymtab::getFuncName(uint64_t FuncMD5Hash) {
+  auto Result =
+      std::lower_bound(HashNameMap.begin(), HashNameMap.end(), FuncMD5Hash,
+                       [](const std::pair<uint64_t, std::string> &LHS,
+                          uint64_t RHS) { return LHS.first < RHS; });
+  if (Result != HashNameMap.end())
+    return Result->second;
+  return StringRef();
+}
 
 struct InstrProfValueSiteRecord {
   /// Value profiling data pairs at a given value site.
@@ -318,7 +329,7 @@ struct InstrProfRecord {
   uint64_t Hash;
   std::vector<uint64_t> Counts;
 
-  typedef std::vector<std::pair<uint64_t, const char *>> ValueMapType;
+  typedef std::vector<std::pair<uint64_t, uint64_t>> ValueMapType;
 
   /// Return the number of value profile kinds with non-zero number
   /// of profile sites.
@@ -343,16 +354,12 @@ struct InstrProfRecord {
   /// Add ValueData for ValueKind at value Site.
   void addValueData(uint32_t ValueKind, uint32_t Site,
                     InstrProfValueData *VData, uint32_t N,
-                    ValueMapType *HashKeys);
+                    ValueMapType *ValueMap);
 
   /// Merge the counts in \p Other into this one.
   /// Optionally scale merged counts by \p Weight.
   instrprof_error merge(InstrProfRecord &Other, uint64_t Weight = 1);
 
-  /// Used by InstrProfWriter: update the value strings to commoned strings in
-  /// the writer instance.
-  void updateStrings(InstrProfStringTable *StrTab);
-
   /// Clear value data entries
   void clearValueData() {
     for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)

include/llvm/ProfileData/InstrProfReader.h

@@ -54,7 +54,7 @@ class InstrProfReader {
   std::error_code LastError;
 
 public:
-  InstrProfReader() : LastError(instrprof_error::success) {}
+  InstrProfReader() : LastError(instrprof_error::success), Symtab() {}
   virtual ~InstrProfReader() {}
 
   /// Read the header.  Required before reading first record.
@@ -65,7 +65,20 @@ public:
   InstrProfIterator begin() { return InstrProfIterator(this); }
   InstrProfIterator end() { return InstrProfIterator(); }
 
- protected:
+  /// Return the PGO symtab. There are three different readers:
+  /// Raw, Text, and Indexed profile readers. The first two types
+  /// of readers are used only by llvm-profdata tool, while the indexed
+  /// profile reader is also used by llvm-cov tool and the compiler (
+  /// backend or frontend). Since creating PGO symtab can create
+  /// significant runtime and memory overhead (as it touches data
+  /// for the whole program), InstrProfSymtab for the indexed profile
+  /// reader should be created on demand and it is recommended to be
+  /// only used for dumping purpose with llvm-proftool, not with the
+  /// compiler.
+  virtual InstrProfSymtab &getSymtab() = 0;
+
+protected:
+  std::unique_ptr<InstrProfSymtab> Symtab;
   /// Set the current std::error_code and return same.
   std::error_code error(std::error_code EC) {
     LastError = EC;
@@ -106,9 +119,6 @@ private:
   /// Iterator over the profile data.
   line_iterator Line;
 
-  // String table for holding a unique copy of all the strings in the profile.
-  InstrProfStringTable StringTable;
-
   TextInstrProfReader(const TextInstrProfReader &) = delete;
   TextInstrProfReader &operator=(const TextInstrProfReader &) = delete;
   std::error_code readValueProfileData(InstrProfRecord &Record);
@@ -121,9 +131,14 @@ public:
   static bool hasFormat(const MemoryBuffer &Buffer);
 
   /// Read the header.
-  std::error_code readHeader() override { return success(); }
+  std::error_code readHeader() override;
   /// Read a single record.
   std::error_code readNextRecord(InstrProfRecord &Record) override;
+
+  InstrProfSymtab &getSymtab() override {
+    assert(Symtab.get());
+    return *Symtab.get();
+  }
 };
 
 /// Reader for the raw instrprof binary format from runtime.
@@ -150,8 +165,6 @@ private:
   uint32_t ValueKindLast;
   uint32_t CurValueDataSize;
 
-  // String table for holding a unique copy of all the strings in the profile.
-  InstrProfStringTable StringTable;
   InstrProfRecord::ValueMapType FunctionPtrToNameMap;
 
   RawInstrProfReader(const RawInstrProfReader &) = delete;
@@ -164,7 +177,13 @@ public:
   std::error_code readHeader() override;
   std::error_code readNextRecord(InstrProfRecord &Record) override;
 
+  InstrProfSymtab &getSymtab() override {
+    assert(Symtab.get());
+    return *Symtab.get();
+  }
+
 private:
+  void createSymtab(InstrProfSymtab &Symtab);
   std::error_code readNextHeader(const char *CurrentPos);
   std::error_code readHeader(const RawInstrProf::Header &Header);
   template <class IntT> IntT swap(IntT Int) const {
@@ -220,7 +239,6 @@ class InstrProfLookupTrait {
   // It should be LE by default, but can be changed
   // for testing purpose.
   support::endianness ValueProfDataEndianness;
-  std::vector<std::pair<uint64_t, const char *>> HashKeys;
 
 public:
   InstrProfLookupTrait(IndexedInstrProf::HashT HashType, unsigned FormatVersion)
@@ -240,9 +258,6 @@ public:
 
   hash_value_type ComputeHash(StringRef K);
 
-  void setHashKeys(std::vector<std::pair<uint64_t, const char *>> HashKeys) {
-    this->HashKeys = std::move(HashKeys);
-  }
   static std::pair<offset_type, offset_type>
   ReadKeyDataLength(const unsigned char *&D) {
     using namespace support;
@@ -277,6 +292,7 @@ struct InstrProfReaderIndexBase {
   virtual void setValueProfDataEndianness(support::endianness Endianness) = 0;
   virtual ~InstrProfReaderIndexBase() {}
   virtual uint64_t getVersion() const = 0;
+  virtual void populateSymtab(InstrProfSymtab &) = 0;
 };
 
 typedef OnDiskIterableChainedHashTable<InstrProfLookupTrait>
@@ -290,9 +306,6 @@ private:
   typename HashTableImpl::data_iterator RecordIterator;
   uint64_t FormatVersion;
 
-  // String table for holding a unique copy of all the strings in the profile.
-  InstrProfStringTable StringTable;
-
 public:
   InstrProfReaderIndex(const unsigned char *Buckets,
                        const unsigned char *const Payload,
@@ -311,6 +324,9 @@ public:
   }
   ~InstrProfReaderIndex() override {}
   uint64_t getVersion() const override { return FormatVersion; }
+  void populateSymtab(InstrProfSymtab &Symtab) override {
+    Symtab.create(HashTable->keys());
+  }
 };
 
 /// Reader for the indexed binary instrprof format.
@@ -362,6 +378,11 @@ public:
   void setValueProfDataEndianness(support::endianness Endianness) {
     Index->setValueProfDataEndianness(Endianness);
   }
+
+  // See description in the base class. This interface is designed
+  // to be used by llvm-profdata (for dumping). Avoid using this when
+  // the client is the compiler.
+  InstrProfSymtab &getSymtab() override;
 };
 
 } // end namespace llvm

include/llvm/ProfileData/InstrProfWriter.h

@@ -29,14 +29,12 @@ public:
   typedef SmallDenseMap<uint64_t, InstrProfRecord, 1> ProfilingData;
 
 private:
-  InstrProfStringTable StringTable;
   StringMap<ProfilingData> FunctionData;
   uint64_t MaxFunctionCount;
+
 public:
   InstrProfWriter() : MaxFunctionCount(0) {}
 
-  /// Update string entries in profile data with references to StringTable.
-  void updateStringTableReferences(InstrProfRecord &I);
   /// Add function counts for the given function. If there are already counts
   /// for this function and the hash and number of counts match, each counter is
   /// summed. Optionally scale counts by \p Weight.
@@ -47,7 +45,7 @@ public:
   void writeText(raw_fd_ostream &OS);
   /// Write \c Record in text format to \c OS
   static void writeRecordInText(const InstrProfRecord &Record,
-                                raw_fd_ostream &OS);
+                                InstrProfSymtab &Symtab, raw_fd_ostream &OS);
   /// Write the profile, returning the raw data. For testing.
   std::unique_ptr<MemoryBuffer> writeBuffer();
 

lib/ProfileData/InstrProf.cpp

@@ -240,18 +240,19 @@ instrprof_error InstrProfRecord::merge(InstrProfRecord &Other,
 
   return Result;
 }
+
 // Map indirect call target name hash to name string.
 uint64_t InstrProfRecord::remapValue(uint64_t Value, uint32_t ValueKind,
-                                     ValueMapType *HashKeys) {
+                                     ValueMapType *ValueMap) {
-  if (!HashKeys)
+  if (!ValueMap)
     return Value;
   switch (ValueKind) {
   case IPVK_IndirectCallTarget: {
     auto Result =
-        std::lower_bound(HashKeys->begin(), HashKeys->end(), Value,
+        std::lower_bound(ValueMap->begin(), ValueMap->end(), Value,
-                         [](const std::pair<uint64_t, const char *> &LHS,
+                         [](const std::pair<uint64_t, uint64_t> &LHS,
                             uint64_t RHS) { return LHS.first < RHS; });
-    if (Result != HashKeys->end())
+    if (Result != ValueMap->end())
       Value = (uint64_t)Result->second;
     break;
   }
@@ -259,21 +260,11 @@ uint64_t InstrProfRecord::remapValue(uint64_t Value, uint32_t ValueKind,
   return Value;
 }
 
-void InstrProfRecord::updateStrings(InstrProfStringTable *StrTab) {
-  if (!StrTab)
-    return;
-
-  Name = StrTab->insertString(Name);
-  for (auto &VSite : IndirectCallSites)
-    for (auto &VData : VSite.ValueData)
-      VData.Value = (uint64_t)StrTab->insertString((const char *)VData.Value);
-}
-
 void InstrProfRecord::addValueData(uint32_t ValueKind, uint32_t Site,
                                    InstrProfValueData *VData, uint32_t N,
-                                   ValueMapType *HashKeys) {
+                                   ValueMapType *ValueMap) {
   for (uint32_t I = 0; I < N; I++) {
-    VData[I].Value = remapValue(VData[I].Value, ValueKind, HashKeys);
+    VData[I].Value = remapValue(VData[I].Value, ValueKind, ValueMap);
   }
   std::vector<InstrProfValueSiteRecord> &ValueSites =
       getValueSitesForKind(ValueKind);
@@ -314,19 +305,8 @@ uint32_t getNumValueDataForSiteInstrProf(const void *R, uint32_t VK,
 void getValueForSiteInstrProf(const void *R, InstrProfValueData *Dst,
                               uint32_t K, uint32_t S,
                               uint64_t (*Mapper)(uint32_t, uint64_t)) {
-  return reinterpret_cast<const InstrProfRecord *>(R)
+  return reinterpret_cast<const InstrProfRecord *>(R)->getValueForSite(
-      ->getValueForSite(Dst, K, S, Mapper);
+      Dst, K, S, Mapper);
-}
-
-uint64_t stringToHash(uint32_t ValueKind, uint64_t Value) {
-  switch (ValueKind) {
-  case IPVK_IndirectCallTarget:
-    return IndexedInstrProf::ComputeHash((const char *)Value);
-    break;
-  default:
-    llvm_unreachable("value kind not handled !");
-  }
-  return Value;
 }
 
 ValueProfData *allocValueProfDataInstrProf(size_t TotalSizeInBytes) {
@@ -342,7 +322,7 @@ static ValueProfRecordClosure InstrProfRecordClosure = {
     getNumValueSitesInstrProf,
     getNumValueDataInstrProf,
     getNumValueDataForSiteInstrProf,
-    stringToHash,
+    0,
     getValueForSiteInstrProf,
     allocValueProfDataInstrProf};
 

lib/ProfileData/InstrProfReader.cpp

@@ -109,6 +109,11 @@ bool TextInstrProfReader::hasFormat(const MemoryBuffer &Buffer) {
                      [](char c) { return ::isprint(c) || ::isspace(c); });
 }
 
+std::error_code TextInstrProfReader::readHeader() {
+  Symtab.reset(new InstrProfSymtab());
+  return success();
+}
+
 std::error_code
 TextInstrProfReader::readValueProfileData(InstrProfRecord &Record) {
 
@@ -126,6 +131,7 @@ TextInstrProfReader::readValueProfileData(InstrProfRecord &Record) {
 
   if (Line.is_at_end())
     return success();
+
   uint32_t NumValueKinds;
   if (Line->getAsInteger(10, NumValueKinds)) {
     // No value profile data
@@ -152,12 +158,13 @@ TextInstrProfReader::readValueProfileData(InstrProfRecord &Record) {
         CHECK_LINE_END(Line);
         std::pair<StringRef, StringRef> VD = Line->split(':');
         uint64_t TakenCount, Value;
-        READ_NUM(VD.second, TakenCount);
+        if (VK == IPVK_IndirectCallTarget) {
-        if (VK == IPVK_IndirectCallTarget)
+          Symtab->addFuncName(VD.first);
-          Value = (uint64_t)StringTable.insertString(VD.first);
+          Value = IndexedInstrProf::ComputeHash(VD.first);
-        else {
+        } else {
           READ_NUM(VD.first, Value);
         }
+        READ_NUM(VD.second, TakenCount);
         CurrentValues.push_back({Value, TakenCount});
         Line++;
       }
@@ -176,11 +183,14 @@ std::error_code TextInstrProfReader::readNextRecord(InstrProfRecord &Record) {
   while (!Line.is_at_end() && (Line->empty() || Line->startswith("#")))
     ++Line;
   // If we hit EOF while looking for a name, we're done.
-  if (Line.is_at_end())
+  if (Line.is_at_end()) {
+    Symtab->finalizeSymtab();
     return error(instrprof_error::eof);
+  }
 
   // Read the function name.
   Record.Name = *Line++;
+  Symtab->addFuncName(Record.Name);
 
   // Read the function hash.
   if (Line.is_at_end())
@@ -213,6 +223,9 @@ std::error_code TextInstrProfReader::readNextRecord(InstrProfRecord &Record) {
   if (std::error_code EC = readValueProfileData(Record))
     return EC;
 
+  // This is needed to avoid two pass parsing because llvm-profdata
+  // does dumping while reading.
+  Symtab->finalizeSymtab();
   return success();
 }
 
@@ -266,8 +279,21 @@ RawInstrProfReader<IntPtrT>::readNextHeader(const char *CurrentPos) {
 }
 
 template <class IntPtrT>
-std::error_code RawInstrProfReader<IntPtrT>::readHeader(
+void RawInstrProfReader<IntPtrT>::createSymtab(InstrProfSymtab &Symtab) {
-    const RawInstrProf::Header &Header) {
+  for (const RawInstrProf::ProfileData<IntPtrT> *I = Data; I != DataEnd; ++I) {
+    StringRef FunctionName(getName(I->NamePtr), swap(I->NameSize));
+    Symtab.addFuncName(FunctionName);
+    const IntPtrT FPtr = swap(I->FunctionPointer);
+    if (!FPtr)
+      continue;
+    Symtab.mapAddress(FPtr, IndexedInstrProf::ComputeHash(FunctionName));
+  }
+  Symtab.finalizeSymtab();
+}
+
+template <class IntPtrT>
+std::error_code
+RawInstrProfReader<IntPtrT>::readHeader(const RawInstrProf::Header &Header) {
   if (swap(Header.Version) != RawInstrProf::Version)
     return error(instrprof_error::unsupported_version);
 
@@ -297,23 +323,12 @@ std::error_code RawInstrProfReader<IntPtrT>::readHeader(
   DataEnd = Data + DataSize;
   CountersStart = reinterpret_cast<const uint64_t *>(Start + CountersOffset);
   NamesStart = Start + NamesOffset;
-  ValueDataStart = reinterpret_cast<const uint8_t*>(Start + ValueDataOffset);
+  ValueDataStart = reinterpret_cast<const uint8_t *>(Start + ValueDataOffset);
   ProfileEnd = Start + ProfileSize;
 
-  FunctionPtrToNameMap.clear();
+  std::unique_ptr<InstrProfSymtab> NewSymtab = make_unique<InstrProfSymtab>();
-  for (const RawInstrProf::ProfileData<IntPtrT> *I = Data; I != DataEnd; ++I) {
+  createSymtab(*NewSymtab.get());
-    const IntPtrT FPtr = swap(I->FunctionPointer);
+  Symtab = std::move(NewSymtab);
-    if (!FPtr)
-      continue;
-    StringRef FunctionName(getName(I->NamePtr), swap(I->NameSize));
-    const char* NameEntryPtr = StringTable.insertString(FunctionName);
-    FunctionPtrToNameMap.push_back(std::pair<const IntPtrT, const char*>
-                                   (FPtr, NameEntryPtr));
-  }
-  std::sort(FunctionPtrToNameMap.begin(), FunctionPtrToNameMap.end(), less_first());
-  FunctionPtrToNameMap.erase(std::unique(FunctionPtrToNameMap.begin(),
-                                         FunctionPtrToNameMap.end()),
-                                         FunctionPtrToNameMap.end());
   return success();
 }
 
@@ -383,7 +398,7 @@ RawInstrProfReader<IntPtrT>::readValueProfilingData(InstrProfRecord &Record) {
   if (VDataPtrOrErr.getError())
     return VDataPtrOrErr.getError();
 
-  VDataPtrOrErr.get()->deserializeTo(Record, &FunctionPtrToNameMap);
+  VDataPtrOrErr.get()->deserializeTo(Record, &Symtab->getAddrHashMap());
   CurValueDataSize = VDataPtrOrErr.get()->getSize();
   return success();
 }
@@ -437,7 +452,7 @@ bool InstrProfLookupTrait::readValueProfilingData(
   if (VDataPtrOrErr.getError())
     return false;
 
-  VDataPtrOrErr.get()->deserializeTo(DataBuffer.back(), &HashKeys);
+  VDataPtrOrErr.get()->deserializeTo(DataBuffer.back(), nullptr);
   D += VDataPtrOrErr.get()->TotalSize;
 
   return true;
@@ -525,16 +540,6 @@ InstrProfReaderIndex<HashTableImpl>::InstrProfReaderIndex(
   HashTable.reset(HashTableImpl::Create(
       Buckets, Payload, Base,
       typename HashTableImpl::InfoType(HashType, Version)));
-  // Form the map of hash values to const char* keys in profiling data.
-  std::vector<std::pair<uint64_t, const char *>> HashKeys;
-  for (auto Key : HashTable->keys()) {
-    const char *KeyTableRef = StringTable.insertString(Key);
-    HashKeys.push_back(std::make_pair(ComputeHash(HashType, Key), KeyTableRef));
-  }
-  std::sort(HashKeys.begin(), HashKeys.end(), less_first());
-  HashKeys.erase(std::unique(HashKeys.begin(), HashKeys.end()), HashKeys.end());
-  // Set the hash key map for the InstrLookupTrait
-  HashTable->getInfoObj().setHashKeys(std::move(HashKeys));
   RecordIterator = HashTable->data_begin();
 }
 
@@ -590,6 +595,17 @@ std::error_code IndexedInstrProfReader::readHeader() {
   return success();
 }
 
+InstrProfSymtab &IndexedInstrProfReader::getSymtab() {
+  if (Symtab.get())
+    return *Symtab.get();
+
+  std::unique_ptr<InstrProfSymtab> NewSymtab = make_unique<InstrProfSymtab>();
+  Index->populateSymtab(*NewSymtab.get());
+
+  Symtab = std::move(NewSymtab);
+  return *Symtab.get();
+}
+
 ErrorOr<InstrProfRecord>
 IndexedInstrProfReader::getInstrProfRecord(StringRef FuncName,
                                            uint64_t FuncHash) {

lib/ProfileData/InstrProfWriter.cpp

@@ -94,13 +94,8 @@ void InstrProfWriter::setValueProfDataEndianness(
   ValueProfDataEndianness = Endianness;
 }
 
-void InstrProfWriter::updateStringTableReferences(InstrProfRecord &I) {
-  I.updateStrings(&StringTable);
-}
-
 std::error_code InstrProfWriter::addRecord(InstrProfRecord &&I,
                                            uint64_t Weight) {
-  updateStringTableReferences(I);
   auto &ProfileDataMap = FunctionData[I.Name];
 
   bool NewFunc;
@@ -188,6 +183,7 @@ static const char *ValueProfKindStr[] = {
 };
 
 void InstrProfWriter::writeRecordInText(const InstrProfRecord &Func,
+                                        InstrProfSymtab &Symtab,
                                         raw_fd_ostream &OS) {
   OS << Func.Name << "\n";
   OS << "# Func Hash:\n" << Func.Hash << "\n";
@@ -215,8 +211,7 @@ void InstrProfWriter::writeRecordInText(const InstrProfRecord &Func,
       std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
       for (uint32_t I = 0; I < ND; I++) {
         if (VK == IPVK_IndirectCallTarget)
-          OS << reinterpret_cast<const char *>(VD[I].Value) << ":"
+          OS << Symtab.getFuncName(VD[I].Value) << ":" << VD[I].Count << "\n";
-             << VD[I].Count << "\n";
         else
           OS << VD[I].Value << ":" << VD[I].Count << "\n";
       }
@@ -227,9 +222,14 @@ void InstrProfWriter::writeRecordInText(const InstrProfRecord &Func,
 }
 
 void InstrProfWriter::writeText(raw_fd_ostream &OS) {
+  InstrProfSymtab Symtab;
+  for (const auto &I : FunctionData)
+    Symtab.addFuncName(I.getKey());
+  Symtab.finalizeSymtab();
+
   for (const auto &I : FunctionData)
     for (const auto &Func : I.getValue())
-      writeRecordInText(Func.second, OS);
+      writeRecordInText(Func.second, Symtab, OS);
 }
 
 std::unique_ptr<MemoryBuffer> InstrProfWriter::writeBuffer() {

tools/llvm-profdata/llvm-profdata.cpp

@@ -267,7 +267,8 @@ static int showInstrProfile(std::string Filename, bool ShowCounts,
     bool doTextFormatDump = (Show && ShowCounts && TextFormat);
 
     if (doTextFormatDump) {
-      InstrProfWriter::writeRecordInText(Func, OS);
+      InstrProfSymtab &Symtab = Reader->getSymtab();
+      InstrProfWriter::writeRecordInText(Func, Symtab, OS);
       continue;
     }
 
@@ -306,6 +307,7 @@ static int showInstrProfile(std::string Filename, bool ShowCounts,
       }
 
       if (ShowIndirectCallTargets) {
+        InstrProfSymtab &Symtab = Reader->getSymtab();
         uint32_t NS = Func.getNumValueSites(IPVK_IndirectCallTarget);
         OS << "    Indirect Target Results: \n";
         for (size_t I = 0; I < NS; ++I) {
@@ -314,7 +316,8 @@ static int showInstrProfile(std::string Filename, bool ShowCounts,
               Func.getValueForSite(IPVK_IndirectCallTarget, I);
           for (uint32_t V = 0; V < NV; V++) {
             OS << "\t[ " << I << ", ";
-            OS << (const char *)VD[V].Value << ", " << VD[V].Count << " ]\n";
+            OS << Symtab.getFuncName(VD[V].Value) << ", " << VD[V].Count
+               << " ]\n";
           }
         }
       }