[clang][serialization] NFCI: Avoid re-reading input file info

[jansvoboda11]

Summary

This patch resolves a FIXME that points out an inefficiency in first deserializing the input file info and the whole input file, which redundantly deserializes the input file info again.

[Bigcheese]

lgtm, thanks for fixing this.

clang/include/clang/Serialization/ASTReader.h

private:
 
   llvm::DenseMap<const Decl *, bool> DefinitionSource;
 
   bool shouldDisableValidationForFile(const serialization::ModuleFile &M) const;
 
   /// Reads a statement from the specified cursor.
   Stmt *ReadStmtFromStream(ModuleFile &F);
 
-  struct InputFileInfo {
-    std::string Filename;
-    uint64_t ContentHash;
-    off_t StoredSize;
-    time_t StoredTime;
-    bool Overridden;
-    bool Transient;
-    bool TopLevelModuleMap;
-  };
-
-  /// Reads the stored information about an input file.
-  InputFileInfo readInputFileInfo(ModuleFile &F, unsigned ID);
+  /// Retrieve the stored information about an input file.
+  serialization::InputFileInfo getInputFileInfo(ModuleFile &F, unsigned ID);
 
   /// Retrieve the file entry and 'overridden' bit for an input
   /// file in the given module file.
   serialization::InputFile getInputFile(ModuleFile &F, unsigned ID,
                                         bool Complain = true);
 
 public:
   void ResolveImportedPath(ModuleFile &M, std::string &Filename);

clang/include/clang/Serialization/ModuleFile.h

enum ModuleKind {
 
   /// File is a PCH file treated as the actual main file.
   MK_MainFile,
 
   /// File is from a prebuilt module path.
   MK_PrebuiltModule
 };
 
+/// The input file info that has been loaded from an AST file.
+struct InputFileInfo {
+  std::string Filename;
+  uint64_t ContentHash;
+  off_t StoredSize;
+  time_t StoredTime;
+  bool Overridden;
+  bool Transient;
+  bool TopLevelModuleMap;
+};
+
 /// The input file that has been loaded from this AST file, along with
 /// bools indicating whether this was an overridden buffer or if it was
 /// out-of-date or not-found.
 class InputFile {
   enum {
     Overridden = 1,
     OutOfDate = 2,
     NotFound = 3
public:
   llvm::BitstreamCursor InputFilesCursor;
 
   /// Offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.
   std::vector<InputFile> InputFilesLoaded;
 
+  /// The input file infos that have been loaded from this AST file.
+  std::vector<InputFileInfo> InputFileInfosLoaded;
+
   // All user input files reside at the index range [0, NumUserInputFiles), and
   // system input files reside at [NumUserInputFiles, InputFilesLoaded.size()).
   unsigned NumUserInputFiles = 0;
 
   /// If non-zero, specifies the time when we last validated input
   /// files.  Zero means we never validated them.
   ///
   /// The time is specified in seconds since the start of the Epoch.

clang/lib/Serialization/ASTReader.cpp

bool ASTReader::shouldDisableValidationForFile(
   case MK_ExplicitModule:
   case MK_PrebuiltModule:
     return bool(DisableValidationKind & DisableValidationForModuleKind::Module);
   }
 
   return false;
 }
 
-ASTReader::InputFileInfo
-ASTReader::readInputFileInfo(ModuleFile &F, unsigned ID) {
+InputFileInfo ASTReader::getInputFileInfo(ModuleFile &F, unsigned ID) {
+  // If this ID is bogus, just return an empty input file.
+  if (ID == 0 || ID > F.InputFileInfosLoaded.size())
+    return InputFileInfo();
+
+  // If we've already loaded this input file, return it.
+  if (!F.InputFileInfosLoaded[ID - 1].Filename.empty())
+    return F.InputFileInfosLoaded[ID - 1];
+
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
   if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
     // FIXME this drops errors on the floor.
     consumeError(std::move(Err));
   }
 
if (Expected<unsigned> Maybe = Cursor.readRecord(Entry.ID, Record))
     assert(static_cast<InputFileRecordTypes>(Maybe.get()) == INPUT_FILE_HASH &&
            "invalid record type for input file hash");
   else {
     // FIXME this drops errors on the floor.
     consumeError(Maybe.takeError());
   }
   R.ContentHash = (static_cast<uint64_t>(Record[1]) << 32) |
                   static_cast<uint64_t>(Record[0]);
+
+  // Note that we've loaded this input file info.
+  F.InputFileInfosLoaded[ID - 1] = R;
   return R;
 }
 
 static unsigned moduleKindForDiagnostic(ModuleKind Kind);
 InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
   // If this ID is bogus, just return an empty input file.
   if (ID == 0 || ID > F.InputFilesLoaded.size())
     return InputFile();
 
   // If we've already loaded this input file, return it.
   if (F.InputFilesLoaded[ID-1].getFile())
     return F.InputFilesLoaded[ID-1];
 
   if (F.InputFilesLoaded[ID-1].isNotFound())
     return InputFile();
 
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
   if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
     // FIXME this drops errors on the floor.
     consumeError(std::move(Err));
   }
 
-  InputFileInfo FI = readInputFileInfo(F, ID);
+  InputFileInfo FI = getInputFileInfo(F, ID);
   off_t StoredSize = FI.StoredSize;
   time_t StoredTime = FI.StoredTime;
   bool Overridden = FI.Overridden;
   bool Transient = FI.Transient;
   StringRef Filename = FI.Filename;
   uint64_t StoredContentHash = FI.ContentHash;
 
   OptionalFileEntryRefDegradesToFileEntryPtr File =
case llvm::BitstreamEntry::EndBlock: {
       if (Listener)
         Listener->visitModuleFile(F.FileName, F.Kind);
 
       if (Listener && Listener->needsInputFileVisitation()) {
         unsigned N = Listener->needsSystemInputFileVisitation() ? NumInputs
                                                                 : NumUserInputs;
         for (unsigned I = 0; I < N; ++I) {
           bool IsSystem = I >= NumUserInputs;
-          InputFileInfo FI = readInputFileInfo(F, I+1);
+          InputFileInfo FI = getInputFileInfo(F, I + 1);
           Listener->visitInputFile(FI.Filename, IsSystem, FI.Overridden,
                                    F.Kind == MK_ExplicitModule ||
                                    F.Kind == MK_PrebuiltModule);
         }
       }
 
       return Success;
     }
case MODULE_MAP_FILE:
       break;
 
     case INPUT_FILE_OFFSETS:
       NumInputs = Record[0];
       NumUserInputs = Record[1];
       F.InputFileOffsets =
           (const llvm::support::unaligned_uint64_t *)Blob.data();
       F.InputFilesLoaded.resize(NumInputs);
+      F.InputFileInfosLoaded.resize(NumInputs);
       F.NumUserInputFiles = NumUserInputs;
       break;
     }
   }
 }
 
 void ASTReader::readIncludedFiles(ModuleFile &F, StringRef Blob,
                                   Preprocessor &PP) {
   using namespace llvm::support;
 
   const unsigned char *D = (const unsigned char *)Blob.data();
   unsigned FileCount = endian::readNext<uint32_t, little, unaligned>(D);
 
   for (unsigned I = 0; I < FileCount; ++I) {
     size_t ID = endian::readNext<uint32_t, little, unaligned>(D);
-    InputFileInfo IFI = readInputFileInfo(F, ID);
+    InputFileInfo IFI = getInputFileInfo(F, ID);
     if (llvm::ErrorOr<const FileEntry *> File =
             PP.getFileManager().getFile(IFI.Filename))
       PP.getIncludedFiles().insert(*File);
   }
 }
 
 llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
                                     unsigned ClientLoadCapabilities) {
void ASTReader::visitInputFiles(serialization::ModuleFile &MF,
   }
 }
 
 void ASTReader::visitTopLevelModuleMaps(
     serialization::ModuleFile &MF,
     llvm::function_ref<void(FileEntryRef FE)> Visitor) {
   unsigned NumInputs = MF.InputFilesLoaded.size();
   for (unsigned I = 0; I < NumInputs; ++I) {
-    InputFileInfo IFI = readInputFileInfo(MF, I + 1);
+    InputFileInfo IFI = getInputFileInfo(MF, I + 1);
     if (IFI.TopLevelModuleMap)
-      // FIXME: This unnecessarily re-reads the InputFileInfo.
       if (auto FE = getInputFile(MF, I + 1).getFile())
         Visitor(*FE);
   }
 }
 
 void ASTReader::finishPendingActions() {
   while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() ||
          !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() ||