[llvm] r372025 - [Coverage] Speed up file-based queries for coverage info, NFC

[Vedant Kumar via llvm-commits]

Author: vedantk
Date: Mon Sep 16 12:08:44 2019
New Revision: 372025

URL: http://llvm.org/viewvc/llvm-project?rev=372025&view=rev
Log:
[Coverage] Speed up file-based queries for coverage info, NFC

Speed up queries for coverage info in a file by reducing the amount of
time spent determining whether a function record corresponds to a file.

This gives a 36% speedup when generating a coverage report for `llc`.
The reduction is entirely in user time.

rdar://54758110

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

Modified:
    llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
    llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp

Modified: llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h?rev=372025&r1=372024&r2=372025&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h (original)
+++ llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h Mon Sep 16 12:08:44 2019
@@ -301,7 +301,12 @@ public:
 struct FunctionRecord {
   /// Raw function name.
   std::string Name;
-  /// Associated files.
+  /// Mapping from FileID (i.e. vector index) to filename. Used to support
+  /// macro expansions within a function in which the macro and function are
+  /// defined in separate files.
+  ///
+  /// TODO: Uniquing filenames across all function records may be a performance
+  /// optimization.
   std::vector<std::string> Filenames;
   /// Regions in the function along with their counts.
   std::vector<CountedRegion> CountedRegions;
@@ -508,6 +513,7 @@ public:
 class CoverageMapping {
   DenseMap<size_t, DenseSet<size_t>> RecordProvenance;
   std::vector<FunctionRecord> Functions;
+  DenseMap<size_t, SmallVector<unsigned, 0>> FilenameHash2RecordIndices;
   std::vector<std::pair<std::string, uint64_t>> FuncHashMismatches;
 
   CoverageMapping() = default;
@@ -516,6 +522,13 @@ class CoverageMapping {
   Error loadFunctionRecord(const CoverageMappingRecord &Record,
                            IndexedInstrProfReader &ProfileReader);
 
+  /// Look up the indices for function records which are at least partially
+  /// defined in the specified file. This is guaranteed to return a superset of
+  /// such records: extra records not in the file may be included if there is
+  /// a hash collision on the filename. Clients must be robust to collisions.
+  ArrayRef<unsigned>
+  getImpreciseRecordIndicesForFilename(StringRef Filename) const;
+
 public:
   CoverageMapping(const CoverageMapping &) = delete;
   CoverageMapping &operator=(const CoverageMapping &) = delete;

Modified: llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp?rev=372025&r1=372024&r2=372025&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp (original)
+++ llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp Mon Sep 16 12:08:44 2019
@@ -194,6 +194,15 @@ void FunctionRecordIterator::skipOtherFi
     *this = FunctionRecordIterator();
 }
 
+ArrayRef<unsigned> CoverageMapping::getImpreciseRecordIndicesForFilename(
+    StringRef Filename) const {
+  size_t FilenameHash = hash_value(Filename);
+  auto RecordIt = FilenameHash2RecordIndices.find(FilenameHash);
+  if (RecordIt == FilenameHash2RecordIndices.end())
+    return {};
+  return RecordIt->second;
+}
+
 Error CoverageMapping::loadFunctionRecord(
     const CoverageMappingRecord &Record,
     IndexedInstrProfReader &ProfileReader) {
@@ -249,6 +258,20 @@ Error CoverageMapping::loadFunctionRecor
     return Error::success();
 
   Functions.push_back(std::move(Function));
+
+  // Performance optimization: keep track of the indices of the function records
+  // which correspond to each filename. This can be used to substantially speed
+  // up queries for coverage info in a file.
+  unsigned RecordIndex = Functions.size() - 1;
+  for (StringRef Filename : Record.Filenames) {
+    auto &RecordIndices = FilenameHash2RecordIndices[hash_value(Filename)];
+    // Note that there may be duplicates in the filename set for a function
+    // record, because of e.g. macro expansions in the function in which both
+    // the macro and the function are defined in the same file.
+    if (RecordIndices.empty() || RecordIndices.back() != RecordIndex)
+      RecordIndices.push_back(RecordIndex);
+  }
+
   return Error::success();
 }
 
@@ -626,7 +649,12 @@ CoverageData CoverageMapping::getCoverag
   CoverageData FileCoverage(Filename);
   std::vector<CountedRegion> Regions;
 
-  for (const auto &Function : Functions) {
+  // Look up the function records in the given file. Due to hash collisions on
+  // the filename, we may get back some records that are not in the file.
+  ArrayRef<unsigned> RecordIndices =
+      getImpreciseRecordIndicesForFilename(Filename);
+  for (unsigned RecordIndex : RecordIndices) {
+    const FunctionRecord &Function = Functions[RecordIndex];
     auto MainFileID = findMainViewFileID(Filename, Function);
     auto FileIDs = gatherFileIDs(Filename, Function);
     for (const auto &CR : Function.CountedRegions)
@@ -646,7 +674,12 @@ CoverageData CoverageMapping::getCoverag
 std::vector<InstantiationGroup>
 CoverageMapping::getInstantiationGroups(StringRef Filename) const {
   FunctionInstantiationSetCollector InstantiationSetCollector;
-  for (const auto &Function : Functions) {
+  // Look up the function records in the given file. Due to hash collisions on
+  // the filename, we may get back some records that are not in the file.
+  ArrayRef<unsigned> RecordIndices =
+      getImpreciseRecordIndicesForFilename(Filename);
+  for (unsigned RecordIndex : RecordIndices) {
+    const FunctionRecord &Function = Functions[RecordIndex];
     auto MainFileID = findMainViewFileID(Filename, Function);
     if (!MainFileID)
       continue;