arphaman created this revision.
arphaman added reviewers: erichkeane, dexonsmith, vsapsai, Bigcheese, 
ilya-biryukov.
Herald added a subscriber: jkorous.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D55455

Files:
  include/clang/Basic/FileManager.h
  include/clang/Basic/FileSystemStatCache.h
  lib/Basic/FileManager.cpp
  lib/Basic/FileSystemStatCache.cpp
  lib/Tooling/Tooling.cpp
  unittests/Basic/FileManagerTest.cpp

Index: unittests/Basic/FileManagerTest.cpp
===================================================================
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -111,7 +111,7 @@
   // 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"));
@@ -121,7 +121,7 @@
 // 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"));
@@ -149,7 +149,7 @@
   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);
@@ -173,7 +173,7 @@
 // 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");
@@ -195,7 +195,7 @@
   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");
@@ -213,7 +213,7 @@
   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);
@@ -260,7 +260,7 @@
   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"));
 }
@@ -273,7 +273,7 @@
   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());
@@ -281,15 +281,6 @@
   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
@@ -300,7 +291,7 @@
   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);
@@ -355,7 +346,7 @@
   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);
Index: lib/Tooling/Tooling.cpp
===================================================================
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -369,7 +369,7 @@
 
   const bool Success = Compiler.ExecuteAction(*ScopedToolAction);
 
-  Files->clearStatCaches();
+  Files->clearStatCache();
   return Success;
 }
 
Index: lib/Basic/FileSystemStatCache.cpp
===================================================================
--- lib/Basic/FileSystemStatCache.cpp
+++ lib/Basic/FileSystemStatCache.cpp
@@ -114,18 +114,17 @@
 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);
-
-  // 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;
+  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).
+    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;
 }
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -63,44 +63,12 @@
 
 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));
+  StatCache = 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.
Index: include/clang/Basic/FileSystemStatCache.h
===================================================================
--- include/clang/Basic/FileSystemStatCache.h
+++ include/clang/Basic/FileSystemStatCache.h
@@ -60,9 +60,6 @@
 class FileSystemStatCache {
   virtual void anchor();
 
-protected:
-  std::unique_ptr<FileSystemStatCache> NextStatCache;
-
 public:
   virtual ~FileSystemStatCache() = default;
 
@@ -88,22 +85,6 @@
                   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
@@ -111,17 +92,6 @@
   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
Index: include/clang/Basic/FileManager.h
===================================================================
--- include/clang/Basic/FileManager.h
+++ include/clang/Basic/FileManager.h
@@ -192,18 +192,10 @@
   ///
   /// \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).
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to