Remove stat cache chaining as it's no longer needed after PTH support has been removed

[arphaman]

Summary

Stat cache chaining was implemented for a StatListener in the PTH writer so that it could write out the stat information to PTH. r348266 removed support for PTH, and it doesn't seem like there are other uses of stat cache chaining. We can remove the chaining support.

[erichkeane]

I've no problem with this, but I don't consider myself 'expert' on this stuff enough to mark 'Approve' without giving others time to take a look. I'd say give this until this time tuesday for the others to take a look, then you can consider this an 'approve'.

I won't mark it as such so as to not scare away other reviews :)

[dexonsmith]

This patch LGTM, but before committing I suggest sending an RFC to cfe-dev and waiting a few days.

[arphaman]

In D55455#1323836, @dexonsmith wrote:

This patch LGTM, but before committing I suggest sending an RFC to cfe-dev and waiting a few days.

I posted to cfe-dev to check if there are any objections.

[ilya-biryukov]

Yay! Anything that makes FileManager and friends simpler LG

[dexonsmith]

It has been about a week since your cfe-dev post with no concerns. I think you can commit.

cfe/trunk/include/clang/Basic/FileManager.h

public:
 
   /// Installs the provided FileSystemStatCache object within
   /// the FileManager.
   ///
   /// Ownership of this object is transferred to the FileManager.
   ///
   /// \param statCache the new stat cache to install. Ownership of this
   /// object is transferred to the FileManager.
-  ///
-  /// \param AtBeginning whether this new stat cache must be installed at the
-  /// beginning of the chain of stat caches. Otherwise, it will be added to
-  /// the end of the chain.
-  void addStatCache(std::unique_ptr<FileSystemStatCache> statCache,
-                    bool AtBeginning = false);
-
-  /// Removes the specified FileSystemStatCache object from the manager.
-  void removeStatCache(FileSystemStatCache *statCache);
+  void setStatCache(std::unique_ptr<FileSystemStatCache> statCache);
 
-  /// Removes all FileSystemStatCache objects from the manager.
-  void clearStatCaches();
+  /// Removes the FileSystemStatCache object from the manager.
+  void clearStatCache();
 
   /// Lookup, cache, and verify the specified directory (real or
   /// virtual).
   ///
   /// This returns NULL if the directory doesn't exist.
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.

cfe/trunk/include/clang/Basic/FileSystemStatCache.h

 };
 
 /// Abstract interface for introducing a FileManager cache for 'stat'
 /// system calls, which is used by precompiled and pretokenized headers to
 /// improve performance.
 class FileSystemStatCache {
   virtual void anchor();
 
-protected:
-  std::unique_ptr<FileSystemStatCache> NextStatCache;
-
 public:
   virtual ~FileSystemStatCache() = default;
 
   enum LookupResult {
     /// We know the file exists and its cached stat data.
     CacheExists,
 
     /// We know that the file doesn't exist.
public:
   /// (not directories).  If it is false this lookup should only return
   /// success for directories (not files).  On a successful file lookup, the
   /// implementation can optionally fill in \p F with a valid \p File object and
   /// the client guarantees that it will close it.
   static bool get(StringRef Path, FileData &Data, bool isFile,
                   std::unique_ptr<llvm::vfs::File> *F,
                   FileSystemStatCache *Cache, llvm::vfs::FileSystem &FS);
 
-  /// Sets the next stat call cache in the chain of stat caches.
-  /// Takes ownership of the given stat cache.
-  void setNextStatCache(std::unique_ptr<FileSystemStatCache> Cache) {
-    NextStatCache = std::move(Cache);
-  }
-
-  /// Retrieve the next stat call cache in the chain.
-  FileSystemStatCache *getNextStatCache() { return NextStatCache.get(); }
-
-  /// Retrieve the next stat call cache in the chain, transferring
-  /// ownership of this cache (and, transitively, all of the remaining caches)
-  /// to the caller.
-  std::unique_ptr<FileSystemStatCache> takeNextStatCache() {
-    return std::move(NextStatCache);
-  }
-
 protected:
   // FIXME: The pointer here is a non-owning/optional reference to the
   // unique_ptr. Optional<unique_ptr<vfs::File>&> might be nicer, but
   // Optional needs some work to support references so this isn't possible yet.
   virtual LookupResult getStat(StringRef Path, FileData &Data, bool isFile,
                                std::unique_ptr<llvm::vfs::File> *F,
                                llvm::vfs::FileSystem &FS) = 0;
-
-  LookupResult statChained(StringRef Path, FileData &Data, bool isFile,
-                           std::unique_ptr<llvm::vfs::File> *F,
-                           llvm::vfs::FileSystem &FS) {
-    if (FileSystemStatCache *Next = getNextStatCache())
-      return Next->getStat(Path, Data, isFile, F, FS);
-
-    // If we hit the end of the list of stat caches to try, just compute and
-    // return it without a cache.
-    return get(Path, Data, isFile, F, nullptr, FS) ? CacheMissing : CacheExists;
-  }
 };
 
 /// A stat "cache" that can be used by FileManager to keep
 /// track of the results of stat() calls that occur throughout the
 /// execution of the front end.
 class MemorizeStatCalls : public FileSystemStatCache {
 public:
   /// The set of stat() calls that have been seen.

cfe/trunk/lib/Basic/FileManager.cpp

FileManager::FileManager(const FileSystemOptions &FSO,
   // If the caller doesn't provide a virtual file system, just grab the real
   // file system.
   if (!this->FS)
     this->FS = llvm::vfs::getRealFileSystem();
 }
 
 FileManager::~FileManager() = default;
 
-void FileManager::addStatCache(std::unique_ptr<FileSystemStatCache> statCache,
-                               bool AtBeginning) {
+void FileManager::setStatCache(std::unique_ptr<FileSystemStatCache> statCache) {
   assert(statCache && "No stat cache provided?");
-  if (AtBeginning || !StatCache.get()) {
-    statCache->setNextStatCache(std::move(StatCache));
-    StatCache = std::move(statCache);
-    return;
-  }
-
-  FileSystemStatCache *LastCache = StatCache.get();
-  while (LastCache->getNextStatCache())
-    LastCache = LastCache->getNextStatCache();
-
-  LastCache->setNextStatCache(std::move(statCache));
 }
 
-void FileManager::removeStatCache(FileSystemStatCache *statCache) {
-  if (!statCache)
-    return;
-
-  if (StatCache.get() == statCache) {
-    // This is the first stat cache.
-    StatCache = StatCache->takeNextStatCache();
-    return;
-  }
-
-  // Find the stat cache in the list.
-  FileSystemStatCache *PrevCache = StatCache.get();
-  while (PrevCache && PrevCache->getNextStatCache() != statCache)
-    PrevCache = PrevCache->getNextStatCache();
-
-  assert(PrevCache && "Stat cache not found for removal");
-  PrevCache->setNextStatCache(statCache->takeNextStatCache());
-}
-
-void FileManager::clearStatCaches() {
-  StatCache.reset();
-}
+void FileManager::clearStatCache() { StatCache.reset(); }
 
 /// Retrieve the directory that the given file name resides in.
 /// Filename can point to either a real file or a virtual file.
 static const DirectoryEntry *getDirectoryFromFile(FileManager &FileMgr,
                                                   StringRef Filename,
                                                   bool CacheFailure) {
   if (Filename.empty())
     return nullptr;

cfe/trunk/lib/Basic/FileSystemStatCache.cpp

bool FileSystemStatCache::get(StringRef Path, FileData &Data, bool isFile,
 
   return false;
 }
 
 MemorizeStatCalls::LookupResult
 MemorizeStatCalls::getStat(StringRef Path, FileData &Data, bool isFile,
                            std::unique_ptr<llvm::vfs::File> *F,
                            llvm::vfs::FileSystem &FS) {
-  LookupResult Result = statChained(Path, Data, isFile, F, FS);
-
+  if (get(Path, Data, isFile, F, nullptr, FS)) {
     // Do not cache failed stats, it is easy to construct common inconsistent
-  // situations if we do, and they are not important for PCH performance (which
-  // currently only needs the stats to construct the initial FileManager
-  // entries).
-  if (Result == CacheMissing)
-    return Result;
+    // situations if we do, and they are not important for PCH performance
+    // (which currently only needs the stats to construct the initial
+    // FileManager entries).
+    return CacheMissing;
+  }
 
   // Cache file 'stat' results and directories with absolutely paths.
   if (!Data.IsDirectory || llvm::sys::path::is_absolute(Path))
     StatCalls[Path] = Data;
 
-  return Result;
+  return CacheExists;
 }

cfe/trunk/lib/Tooling/Tooling.cpp

bool FrontendActionFactory::runInvocation(
   Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
   if (!Compiler.hasDiagnostics())
     return false;
 
   Compiler.createSourceManager(*Files);
 
   const bool Success = Compiler.ExecuteAction(*ScopedToolAction);
 
-  Files->clearStatCaches();
+  Files->clearStatCache();
   return Success;
 }
 
 ClangTool::ClangTool(const CompilationDatabase &Compilations,
                      ArrayRef<std::string> SourcePaths,
                      std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                      IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS)
     : Compilations(Compilations), SourcePaths(SourcePaths),

cfe/trunk/unittests/Basic/FileManagerTest.cpp

 }
 
 // Before any virtual file is added, no virtual directory exists.
 TEST_F(FileManagerTest, NoVirtualDirectoryExistsBeforeAVirtualFileIsAdded) {
   // An empty FakeStatCache causes all stat calls made by the
   // FileManager to report "file/directory doesn't exist".  This
   // avoids the possibility of the result of this test being affected
   // by what's in the real file system.
-  manager.addStatCache(llvm::make_unique<FakeStatCache>());
+  manager.setStatCache(llvm::make_unique<FakeStatCache>());
 
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir"));
   EXPECT_EQ(nullptr, manager.getDirectory("virtual"));
 }
 
 // When a virtual file is added, all of its ancestors should be created.
 TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
   // Fake an empty real file system.
-  manager.addStatCache(llvm::make_unique<FakeStatCache>());
+  manager.setStatCache(llvm::make_unique<FakeStatCache>());
 
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
 
   const DirectoryEntry *dir = manager.getDirectory("virtual/dir");
   ASSERT_TRUE(dir != nullptr);
   EXPECT_EQ("virtual/dir", dir->getName());
 

 
 #ifdef _WIN32
   const char *DirName = "C:.";
   const char *FileName = "C:test";
   statCache->InjectDirectory(DirName, 44);
   statCache->InjectFile(FileName, 45);
 #endif
 
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   const FileEntry *file = manager.getFile("/tmp/test");
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
   EXPECT_EQ("/tmp/test", file->getName());
 
   const DirectoryEntry *dir = file->getDir();
   ASSERT_TRUE(dir != nullptr);
   EXPECT_EQ("/tmp", dir->getName());
 
 #ifdef _WIN32
   file = manager.getFile(FileName);
   ASSERT_TRUE(file != NULL);
 
   dir = file->getDir();
   ASSERT_TRUE(dir != NULL);
   EXPECT_EQ(DirName, dir->getName());
 #endif
 }
 
 // getFile() returns non-NULL if a virtual file exists at the given path.
 TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
   // Fake an empty real file system.
-  manager.addStatCache(llvm::make_unique<FakeStatCache>());
+  manager.setStatCache(llvm::make_unique<FakeStatCache>());
 
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
   const FileEntry *file = manager.getFile("virtual/dir/bar.h");
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
   EXPECT_EQ("virtual/dir/bar.h", file->getName());
 
   const DirectoryEntry *dir = file->getDir();
   ASSERT_TRUE(dir != nullptr);
   EXPECT_EQ("virtual/dir", dir->getName());
 }
 
 // getFile() returns different FileEntries for different paths when
 // there's no aliasing.
 TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) {
   // Inject two fake files into the file system.  Different inodes
   // mean the files are not symlinked together.
   auto statCache = llvm::make_unique<FakeStatCache>();
   statCache->InjectDirectory(".", 41);
   statCache->InjectFile("foo.cpp", 42);
   statCache->InjectFile("bar.cpp", 43);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   const FileEntry *fileFoo = manager.getFile("foo.cpp");
   const FileEntry *fileBar = manager.getFile("bar.cpp");
   ASSERT_TRUE(fileFoo != nullptr);
   ASSERT_TRUE(fileFoo->isValid());
   ASSERT_TRUE(fileBar != nullptr);
   ASSERT_TRUE(fileBar->isValid());
   EXPECT_NE(fileFoo, fileBar);
 }
 
 // getFile() returns NULL if neither a real file nor a virtual file
 // exists at the given path.
 TEST_F(FileManagerTest, getFileReturnsNULLForNonexistentFile) {
   // Inject a fake foo.cpp into the file system.
   auto statCache = llvm::make_unique<FakeStatCache>();
   statCache->InjectDirectory(".", 41);
   statCache->InjectFile("foo.cpp", 42);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   // Create a virtual bar.cpp file.
   manager.getVirtualFile("bar.cpp", 200, 0);
 
   const FileEntry *file = manager.getFile("xyz.txt");
   EXPECT_EQ(nullptr, file);
 }
 

 
 // getFile() returns the same FileEntry for real files that are aliases.
 TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) {
   // Inject two real files with the same inode.
   auto statCache = llvm::make_unique<FakeStatCache>();
   statCache->InjectDirectory("abc", 41);
   statCache->InjectFile("abc/foo.cpp", 42);
   statCache->InjectFile("abc/bar.cpp", 42);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
 }
 
 // getFile() returns the same FileEntry for virtual files that have
 // corresponding real files that are aliases.
 TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
   // Inject two real files with the same inode.
   auto statCache = llvm::make_unique<FakeStatCache>();
   statCache->InjectDirectory("abc", 41);
   statCache->InjectFile("abc/foo.cpp", 42);
   statCache->InjectFile("abc/bar.cpp", 42);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   ASSERT_TRUE(manager.getVirtualFile("abc/foo.cpp", 100, 0)->isValid());
   ASSERT_TRUE(manager.getVirtualFile("abc/bar.cpp", 200, 0)->isValid());
 
   EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
 }
 
-TEST_F(FileManagerTest, addRemoveStatCache) {
-  manager.addStatCache(llvm::make_unique<FakeStatCache>());
-  auto statCacheOwner = llvm::make_unique<FakeStatCache>();
-  auto *statCache = statCacheOwner.get();
-  manager.addStatCache(std::move(statCacheOwner));
-  manager.addStatCache(llvm::make_unique<FakeStatCache>());
-  manager.removeStatCache(statCache);
-}
-
 // getFile() Should return the same entry as getVirtualFile if the file actually
 // is a virtual file, even if the name is not exactly the same (but is after
 // normalisation done by the file system, like on Windows). This can be checked
 // here by checking the size.
 TEST_F(FileManagerTest, getVirtualFileWithDifferentName) {
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique<FakeStatCache>();
   statCache->InjectDirectory("c:\\tmp", 42);
   statCache->InjectFile("c:\\tmp\\test", 43);
 
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   // Inject the virtual file:
   const FileEntry *file1 = manager.getVirtualFile("c:\\tmp\\test", 123, 1);
   ASSERT_TRUE(file1 != nullptr);
   ASSERT_TRUE(file1->isValid());
   EXPECT_EQ(43U, file1->getUniqueID().getFile());
   EXPECT_EQ(123, file1->getSize());
 
#endif
   FileSystemOptions Opts;
   FileManager Manager(Opts, FS);
 
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique<FakeStatCache>();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
 
-  Manager.addStatCache(std::move(statCache));
+  Manager.setStatCache(std::move(statCache));
 
   // Check for real path.
   const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
   SmallString<64> ExpectedResult = CustomWorkingDir;
 
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
 } // anonymous namespace