Commit 83f4c3af authored by Volodymyr Sapsai's avatar Volodymyr Sapsai
Browse files

[modules] Do not cache invalid state for modules that we attempted to load.

Partially reverts 0a2be46c as it turned
out to cause redundant module rebuilds in multi-process incremental builds.
When a module was getting out of date, all compilation processes started at the
same time were marking it as `ToBuild`. So each process was building the same
module instead of checking if it was built by someone else and using that
result. In addition to the work duplication, contention on the same .pcm file
wasn't making builds faster.

Note that for a single-process build this change would cause redundant module
reads and validations. But reading a module is faster than building it and
multi-process builds are more common than single-process. So I'm willing to
make such a trade-off.

rdar://problem/54395127

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D72860
parent f55ab6f9
Loading
Loading
Loading
Loading
+8 −34
Original line number Diff line number Diff line
@@ -45,61 +45,35 @@ class InMemoryModuleCache : public llvm::RefCountedBase<InMemoryModuleCache> {
  llvm::StringMap<PCM> PCMs;

public:
  /// There are four states for a PCM.  It must monotonically increase.
  ///
  ///  1. Unknown: the PCM has neither been read from disk nor built.
  ///  2. Tentative: the PCM has been read from disk but not yet imported or
  ///     built.  It might work.
  ///  3. ToBuild: the PCM read from disk did not work but a new one has not
  ///     been built yet.
  ///  4. Final: indicating that the current PCM was either built in this
  ///     process or has been successfully imported.
  enum State { Unknown, Tentative, ToBuild, Final };

  /// Get the state of the PCM.
  State getPCMState(llvm::StringRef Filename) const;

  /// Store the PCM under the Filename.
  ///
  /// \pre state is Unknown
  /// \post state is Tentative
  /// \pre PCM for the same Filename shouldn't be in cache already.
  /// \return a reference to the buffer as a convenience.
  llvm::MemoryBuffer &addPCM(llvm::StringRef Filename,
                             std::unique_ptr<llvm::MemoryBuffer> Buffer);

  /// Store a just-built PCM under the Filename.
  /// Store a final PCM under the Filename.
  ///
  /// \pre state is Unknown or ToBuild.
  /// \pre state is not Tentative.
  /// \pre PCM for the same Filename shouldn't be in cache already.
  /// \return a reference to the buffer as a convenience.
  llvm::MemoryBuffer &addBuiltPCM(llvm::StringRef Filename,
  llvm::MemoryBuffer &addFinalPCM(llvm::StringRef Filename,
                                  std::unique_ptr<llvm::MemoryBuffer> Buffer);

  /// Try to remove a buffer from the cache.  No effect if state is Final.
  /// Try to remove a PCM from the cache.  No effect if it is Final.
  ///
  /// \pre state is Tentative/Final.
  /// \post Tentative => ToBuild or Final => Final.
  /// \return false on success, i.e. if Tentative => ToBuild.
  bool tryToDropPCM(llvm::StringRef Filename);
  /// \return false on success.
  bool tryToRemovePCM(llvm::StringRef Filename);

  /// Mark a PCM as final.
  ///
  /// \pre state is Tentative or Final.
  /// \post state is Final.
  void finalizePCM(llvm::StringRef Filename);

  /// Get a pointer to the pCM if it exists; else nullptr.
  /// Get a pointer to the PCM if it exists; else nullptr.
  llvm::MemoryBuffer *lookupPCM(llvm::StringRef Filename) const;

  /// Check whether the PCM is final and has been shown to work.
  ///
  /// \return true iff state is Final.
  bool isPCMFinal(llvm::StringRef Filename) const;

  /// Check whether the PCM is waiting to be built.
  ///
  /// \return true iff state is ToBuild.
  bool shouldBuildPCM(llvm::StringRef Filename) const;
};

} // end namespace clang
+1 −1
Original line number Diff line number Diff line
@@ -4503,7 +4503,7 @@ ASTReader::ReadASTCore(StringRef FileName,
    if (ShouldFinalizePCM)
      MC.finalizePCM(FileName);
    else
      MC.tryToDropPCM(FileName);
      MC.tryToRemovePCM(FileName);
  });
  ModuleFile &F = *M;
  BitstreamCursor &Stream = F.Stream;
+1 −1
Original line number Diff line number Diff line
@@ -4304,7 +4304,7 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef,
  WritingAST = false;
  if (ShouldCacheASTInMemory) {
    // Construct MemoryBuffer and update buffer manager.
    ModuleCache.addBuiltPCM(OutputFile,
    ModuleCache.addFinalPCM(OutputFile,
                            llvm::MemoryBuffer::getMemBufferCopy(
                                StringRef(Buffer.begin(), Buffer.size())));
  }
+8 −21
Original line number Diff line number Diff line
@@ -11,16 +11,6 @@

using namespace clang;

InMemoryModuleCache::State
InMemoryModuleCache::getPCMState(llvm::StringRef Filename) const {
  auto I = PCMs.find(Filename);
  if (I == PCMs.end())
    return Unknown;
  if (I->second.IsFinal)
    return Final;
  return I->second.Buffer ? Tentative : ToBuild;
}

llvm::MemoryBuffer &
InMemoryModuleCache::addPCM(llvm::StringRef Filename,
                            std::unique_ptr<llvm::MemoryBuffer> Buffer) {
@@ -30,11 +20,11 @@ InMemoryModuleCache::addPCM(llvm::StringRef Filename,
}

llvm::MemoryBuffer &
InMemoryModuleCache::addBuiltPCM(llvm::StringRef Filename,
InMemoryModuleCache::addFinalPCM(llvm::StringRef Filename,
                                 std::unique_ptr<llvm::MemoryBuffer> Buffer) {
  auto &PCM = PCMs[Filename];
  assert(!PCM.IsFinal && "Trying to override finalized PCM?");
  assert(!PCM.Buffer && "Trying to override tentative PCM?");
  assert(!PCM.Buffer && "Already has a non-final PCM");
  PCM.Buffer = std::move(Buffer);
  PCM.IsFinal = true;
  return *PCM.Buffer;
@@ -49,24 +39,21 @@ InMemoryModuleCache::lookupPCM(llvm::StringRef Filename) const {
}

bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const {
  return getPCMState(Filename) == Final;
}

bool InMemoryModuleCache::shouldBuildPCM(llvm::StringRef Filename) const {
  return getPCMState(Filename) == ToBuild;
  auto I = PCMs.find(Filename);
  if (I == PCMs.end())
    return false;
  return I->second.IsFinal;
}

bool InMemoryModuleCache::tryToDropPCM(llvm::StringRef Filename) {
bool InMemoryModuleCache::tryToRemovePCM(llvm::StringRef Filename) {
  auto I = PCMs.find(Filename);
  assert(I != PCMs.end() && "PCM to remove is unknown...");

  auto &PCM = I->second;
  assert(PCM.Buffer && "PCM to remove is scheduled to be built...");

  if (PCM.IsFinal)
    return true;

  PCM.Buffer.reset();
  PCMs.erase(I);
  return false;
}

+4 −7
Original line number Diff line number Diff line
@@ -163,7 +163,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
  // Load the contents of the module
  if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
    // The buffer was already provided for us.
    NewModule->Buffer = &ModuleCache->addBuiltPCM(FileName, std::move(Buffer));
    NewModule->Buffer = &ModuleCache->addFinalPCM(FileName, std::move(Buffer));
    // Since the cached buffer is reused, it is safe to close the file
    // descriptor that was opened while stat()ing the PCM in
    // lookupModuleFile() above, it won't be needed any longer.
@@ -173,11 +173,6 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
    NewModule->Buffer = Buffer;
    // As above, the file descriptor is no longer needed.
    Entry->closeFile();
  } else if (getModuleCache().shouldBuildPCM(FileName)) {
    // Report that the module is out of date, since we tried (and failed) to
    // import it earlier.
    Entry->closeFile();
    return OutOfDate;
  } else {
    // Open the AST file.
    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
@@ -185,7 +180,9 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
      Buf = llvm::MemoryBuffer::getSTDIN();
    } else {
      // Get a buffer of the file and close the file descriptor when done.
      Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
      // The file is volatile because in a parallel build we expect multiple
      // compiler processes to use the same module file rebuilding it if needed.
      Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
    }

    if (!Buf) {
Loading