[clangd] Extract common file-caching logic from ConfigProvider.

[sammccall]

Summary

The plan is to use this to use this for .clang-format, .clang-tidy, and
compile_commands.json. (Currently the former two are reparsed every
time, and the latter is cached forever and changes are never seen).

[sammccall]

To give a bit of context here: the end-goal here is hot-reload of compile_commands.json. (This covers our *two* top bugs: #192 and #83).

I'm very happy with the hot-reloading of config files (.clangd etc) and I think it's a good fit to generalize.
In particular, config parsing shouldn't be repeated unneccesarily as it has side-effects (diagnostics). Similarly, compile_commands parsing shouldn't be repeated as it's expensive (the file is often huge).
So this is better than other approaches like caching at the FS level.

We could reasonably apply this to .clang-format and .clang-tidy as well. Currently we stat up the directory chain, and then read and parse these every time. It's not very expensive as these are small files, but it's also not free.

In all of these cases, the costs are going to be in eschewing the simple APIs that hide all the IO. For compile_commands.json, we need to extend or stop using the generic compilation-database plugin mechanism. For clang-tidy/format we need to introduce a tree-shaped cache (we have one for CDBs already).

[sammccall]

@kbobyrev ping... I think we do actually want to land this, for use with .clang-tidy files after D91029

[njames93]

In D88172#2414249, @sammccall wrote:

@kbobyrev ping... I think we do actually want to land this, for use with .clang-tidy files after D91029

Yes, I was looking at copying the config cache for use with .clang-tidy

clang-tools-extra/clangd/support/FileCache.h

44

nit: Is it more descriptive to have this spelled as PathRef, same for path() below.

[kbobyrev]

Sorry for such a tremendous delay.

Mostly looks good functionally, I just have a few questions about semantics and several stylistic nits.

Also, thanks for the amount of documentation - it makes it really nice to navigate around and review.

clang-tools-extra/clangd/support/FileCache.cpp

19

Uhhh. Doesn't Clang-Tidy think this is a bad idea? IIUC this is not UB but it looks quite scary regardless. Maybe use std::numeric_limits<uint64_t>::max() here?

45

Wait, I thought ValidTime is the last time point when the file could be correctly parsed (i.e. the file was valid). But if the parsing could not be done then this would be bumped regardless, right? So what's the "validity" semantics here?

clang-tools-extra/clangd/support/FileCache.h

24

s/live/fresh?

59

So the client is responsible for keeping FreshTime updated, right? I think it might be worth mentioning in the comments.

71

I assume MTime is ModifiedTime - took me some time to realize it (getting into the code), maybe expand or add some comment. Also, why are the types different with ValidTime?

clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp

16

Please sort the includes here (clangd should have a Code Action here).

[njames93]

clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp

16

The includes are sorted, this is a bug in the include order check that was fixed in D91602, but the pre merge bot isnt using the updated version.

[kbobyrev]

clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp

16

Ahh, I didn't know that and I had some of the valid warnings in my patches. I see, thanks!

[sammccall]

clang-tools-extra/clangd/support/FileCache.cpp

19

Done.
(FWIW I prefer the -1 idiom as you don't have to repeat the type so there's no chance of a mismatch)

45

Ah, "valid" refers to the cache, not the file. The cache is valid if it represents what's on disk, whether that's good, bad or missing. (Note that this class doesn't actually know anything about the semantics of the file content, or whether it's valid).

Added a comment to the member.

clang-tools-extra/clangd/support/FileCache.h

24

I guess it depends which way you look at it:
You want the *configuration file* to be "live" in the sense that it takes immediate effect.
You want the *configuration* to be "fresh" in the sense that it reflects the latest source from somewhere.

71

Renamed to ModifiedTime. (mtime is a term of art: see mtime/ctime/atime. But spelling it out is nearly as precise and less cryptic).

The types are different because they are different coordinate systems: steady_clock is approximately "nanos since boot" which is the best way to compare timestamps captured within a program, and system_clock has semantics that match mtime stored in the filesystem (though actually we just compare for equality, so it's just the most convenient type).

Changed ModifiedTime to spell the underlying std type directly to make this difference clearer (though the API we get it from uses the LLVM typedef).

[sammccall]

(oops, missed a merge conflict. New snapshot shortly)

[sammccall]

Address review comments

[sammccall]

clang-tools-extra/clangd/support/FileCache.h

59

I'm not really following - FreshTime is a parameter and must be provided each time, so I'm not sure what "keep it updated" means.

It's true that clients likely want to pass now() + some_duration, I've added an example explaining that to the docs.
But I don't think "if you store the value of now() for a long time and use it later, it may refer to a time way in the past" isn't a trap we need to warn against.

clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp

16

I can apply the fix, but then arc diff wants to revert it. I don't think this is important enough to fight the tools over.

[kbobyrev]

LGTM with std::numeric_limits in the second place :) Thanks!

clang-tools-extra/clangd/support/FileCache.cpp

19

Yeah I guess one could use reflection for that but that but that's more code.

22

I think makes sense to do std::numeric_limits() here, too for consistency?

[njames93]

clang-tools-extra/clangd/support/FileCache.cpp

60

Is tracking the size really that important?
Modified time should be good enough. If someone modifies the file but ensures the mtime doesnt change you have to think there is a reason for that. Also a modification could preserve file size, in which case that wouldn't track anything.

[sammccall]

clang-tools-extra/clangd/support/FileCache.cpp

60

If the content changes without changing mtime, and we don't detect it using size, we're stuck with an invalid cache until it changes again (which could be forever).

you have to think there is a reason for that

My best guess is if the FS has a low resolution (e.g. FAT32, though hopefully people aren't putting config files on USB sticks...) or the file was written twice e.g. by a script fast enough to beat the timer. In both cases I'd want the changes to be revealed.
What's the reason you're thinking of?

Also a modification could preserve file size, in which case that wouldn't track anything

Yup, I don't have a good answer for that case. Hopefully it's very rare. I don't think we should let the perfect be the enemy of the good.

clang-tools-extra/clangd/ConfigProvider.h

 
 /// Describes the context used to evaluate configuration fragments.
 struct Params {
   /// Absolute path to a source file we're applying the config to. Unix slashes.
   /// Empty if not configuring a particular file.
   llvm::StringRef Path;
   /// Hint that stale data is OK to improve performance (e.g. avoid IO).
   /// FreshTime sets a bound for how old the data can be.
-  /// If not set, providers should validate caches against the data source.
-  llvm::Optional<std::chrono::steady_clock::time_point> FreshTime;
+  /// By default, providers should validate caches against the data source.
+  std::chrono::steady_clock::time_point FreshTime =
+      std::chrono::steady_clock::time_point::max();
 };
 
 /// Used to report problems in parsing or interpreting a config.
 /// Errors reflect structurally invalid config that should be user-visible.
 /// Warnings reflect e.g. unknown properties that are recoverable.
 using DiagnosticCallback = llvm::function_ref<void(const llvm::SMDiagnostic &)>;
 
 /// A chunk of configuration that has been fully analyzed and is ready to apply.

clang-tools-extra/clangd/ConfigProvider.cpp

 //===--- ConfigProvider.cpp - Loading of user configuration ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ConfigProvider.h"
 #include "Config.h"
 #include "ConfigFragment.h"
+#include "support/FileCache.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 #include <chrono>
 #include <mutex>
 #include <string>
 
 namespace clang {
 namespace clangd {
 namespace config {
 
 // Threadsafe cache around reading a YAML config file from disk.
-class FileConfigCache {
-  std::mutex Mu;
-  std::chrono::steady_clock::time_point ValidTime = {};
-  llvm::SmallVector<CompiledFragment, 1> CachedValue;
-  llvm::sys::TimePoint<> MTime = {};
-  unsigned Size = -1;
-
-  // Called once we are sure we want to read the file.
-  // REQUIRES: Cache keys are set. Mutex must be held.
-  void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
-    CachedValue.clear();
-
-    auto Buf = FS.getBufferForFile(Path);
-    // If we failed to read (but stat succeeded), don't cache failure.
-    if (!Buf) {
-      Size = -1;
-      MTime = {};
-      return;
-    }
-
-    // If file changed between stat and open, we don't know its mtime.
-    // For simplicity, don't cache the value in this case (use a bad key).
-    if (Buf->get()->getBufferSize() != Size) {
-      Size = -1;
-      MTime = {};
-    }
-
-    // Finally parse and compile the actual fragments.
-    for (auto &Fragment :
-         Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC)) {
-      Fragment.Source.Directory = Directory;
-      CachedValue.push_back(std::move(Fragment).compile(DC));
-    }
-  }
-
-public:
-  // Must be set before the cache is used. Not a constructor param to allow
-  // computing ancestor-relative paths to be deferred.
-  std::string Path;
-  // Directory associated with this fragment.
+class FileConfigCache : public FileCache {
+  mutable llvm::SmallVector<CompiledFragment, 1> CachedValue;
   std::string Directory;
 
-  // Retrieves up-to-date config fragments from disk.
-  // A cached result may be reused if the mtime and size are unchanged.
-  // (But several concurrent read()s can miss the cache after a single change).
-  // Future performance ideas:
-  // - allow caches to be reused based on short elapsed walltime
-  // - allow latency-sensitive operations to skip revalidating the cache
-  void read(const ThreadsafeFS &TFS, DiagnosticCallback DC,
-            llvm::Optional<std::chrono::steady_clock::time_point> FreshTime,
-            std::vector<CompiledFragment> &Out) {
-    std::lock_guard<std::mutex> Lock(Mu);
-    // We're going to update the cache and return whatever's in it.
-    auto Return = llvm::make_scope_exit(
-        [&] { llvm::copy(CachedValue, std::back_inserter(Out)); });
-
-    // Return any sufficiently recent result without doing any further work.
-    if (FreshTime && ValidTime >= FreshTime)
-      return;
-
-    // Ensure we bump the ValidTime at the end to allow for reuse.
-    auto MarkTime = llvm::make_scope_exit(
-        [&] { ValidTime = std::chrono::steady_clock::now(); });
+public:
+  FileConfigCache(llvm::StringRef Path, llvm::StringRef Directory)
+      : FileCache(Path), Directory(Directory) {}
 
-    // Stat is cheaper than opening the file, it's usually unchanged.
-    assert(llvm::sys::path::is_absolute(Path));
-    auto FS = TFS.view(/*CWD=*/llvm::None);
-    auto Stat = FS->status(Path);
-    // If there's no file, the result is empty. Ensure we have an invalid key.
-    if (!Stat || !Stat->isRegularFile()) {
-      MTime = {};
-      Size = -1;
+  void get(const ThreadsafeFS &TFS, DiagnosticCallback DC,
+           std::chrono::steady_clock::time_point FreshTime,
+           std::vector<CompiledFragment> &Out) const {
+    read(
+        TFS, FreshTime,
+        [&](llvm::Optional<llvm::StringRef> Data) {
           CachedValue.clear();
-      return;
+          if (Data)
+            for (auto &Fragment : Fragment::parseYAML(*Data, path(), DC)) {
+              Fragment.Source.Directory = Directory;
+              CachedValue.push_back(std::move(Fragment).compile(DC));
             }
-    // If the modified-time and size match, assume the content does too.
-    if (Size == Stat->getSize() && MTime == Stat->getLastModificationTime())
-      return;
-
-    // OK, the file has actually changed. Update cache key, compute new value.
-    Size = Stat->getSize();
-    MTime = Stat->getLastModificationTime();
-    fillCacheFromDisk(*FS, DC);
+        },
+        [&]() { llvm::copy(CachedValue, std::back_inserter(Out)); });
   }
 };
 
 std::unique_ptr<Provider> Provider::fromYAMLFile(llvm::StringRef AbsPath,
                                                  llvm::StringRef Directory,
                                                  const ThreadsafeFS &FS) {
   class AbsFileProvider : public Provider {
     mutable FileConfigCache Cache; // threadsafe
     const ThreadsafeFS &FS;
 
     std::vector<CompiledFragment>
     getFragments(const Params &P, DiagnosticCallback DC) const override {
       std::vector<CompiledFragment> Result;
-      Cache.read(FS, DC, P.FreshTime, Result);
+      Cache.get(FS, DC, P.FreshTime, Result);
       return Result;
     };
 
   public:
     AbsFileProvider(llvm::StringRef Path, llvm::StringRef Directory,
                     const ThreadsafeFS &FS)
-        : FS(FS) {
+        : Cache(Path, Directory), FS(FS) {
       assert(llvm::sys::path::is_absolute(Path));
-      Cache.Path = Path.str();
-      Cache.Directory = Directory.str();
     }
   };
 
   return std::make_unique<AbsFileProvider>(AbsPath, Directory, FS);
 }
 
 std::unique_ptr<Provider>
 Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath,
getFragments(const Params &P, DiagnosticCallback DC) const override {
           continue;
         Ancestors.emplace_back(Parent.begin(), I->end() - Parent.begin());
       }
       // Ensure corresponding cache entries exist in the map.
       llvm::SmallVector<FileConfigCache *, 8> Caches;
       {
         std::lock_guard<std::mutex> Lock(Mu);
         for (llvm::StringRef Ancestor : Ancestors) {
-          auto R = Cache.try_emplace(Ancestor);
+          auto It = Cache.find(Ancestor);
           // Assemble the actual config file path only once.
-          if (R.second) {
+          if (It == Cache.end()) {
             llvm::SmallString<256> ConfigPath = Ancestor;
             path::append(ConfigPath, RelPath);
-            R.first->second.Path = ConfigPath.str().str();
-            R.first->second.Directory = Ancestor.str();
+            It = Cache.try_emplace(Ancestor, ConfigPath.str(), Ancestor).first;
           }
-          Caches.push_back(&R.first->second);
+          Caches.push_back(&It->second);
         }
       }
       // Finally query each individual file.
       // This will take a (per-file) lock for each file that actually exists.
       std::vector<CompiledFragment> Result;
-      for (FileConfigCache *Cache : Caches) {
-        Cache->read(FS, DC, P.FreshTime, Result);
-      }
+      for (FileConfigCache *Cache : Caches)
+        Cache->get(FS, DC, P.FreshTime, Result);
       return Result;
     };
 
   public:
     RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS)
         : RelPath(RelPath), FS(FS) {
       assert(llvm::sys::path::is_relative(RelPath));
     }

clang-tools-extra/clangd/support/CMakeLists.txt

 if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB OR NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
   list(APPEND CLANGD_ATOMIC_LIB "atomic")
 endif()
 
 include_directories(..)
 add_clang_library(clangdSupport
   Cancellation.cpp
   Context.cpp
+  FileCache.cpp
   Logger.cpp
   Markup.cpp
   MemoryTree.cpp
   Shutdown.cpp
   Threading.cpp
   ThreadsafeFS.cpp
   Trace.cpp
 
   LINK_LIBS
   ${LLVM_PTHREAD_LIB}
   ${CLANGD_ATOMIC_LIB}
   )

clang-tools-extra/clangd/support/FileCache.h

+//===--- FileCache.h - Revalidating cache of data from disk ------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_FILECACHE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_FILECACHE_H
+
+#include "Path.h"
+#include "ThreadsafeFS.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Chrono.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <mutex>
+
+namespace clang {
+namespace clangd {
+
+/// Base class for threadsafe cache of data read from a file on disk.
+///
+/// We want configuration files to be "live" as much as possible.
+/// Reading them every time is simplest, but caching solves a few problems:
+///  - reading and parsing is cheap but not free (and happens on hot paths)
+///  - we can ignore invalid data and use the old value (we may see truncated
+///    compile_commands.json from non-atomic writers)
+///  - we avoid reporting the same errors repeatedly
+///
+/// We still read and parse the data synchronously on demand, but skip as much
+/// work as possible:
+///  - if not enough wall-time has elapsed, assume the data is still up-to-date
+///  - if we stat the file and it has the same mtime + size, don't read it
+///  - obviously we only have to parse when we re-read the file
+/// (Tracking OS change events is an alternative, but difficult to do portably.)
+///
+/// Caches for particular data (e.g. compilation databases) should inherit and:
+///  - add mutable storage for the cached parsed data
+///  - add a public interface implemented on top of read()
+class FileCache {
+protected:
+  // Path must be absolute.
+  FileCache(PathRef Path);
+
+  // Updates the cached value if needed, then provides threadsafe access to it.
+  //
+  // Specifically:
+  // - Parse() may be called (if the cache was not up-to-date)
+  //   The lock is held, so cache storage may be safely written.
+  //   Parse(None) means the file doesn't exist.
+  // - Read() will always be called, to provide access to the value.
+  //   The lock is again held, so the value can be copied or used.
+  //
+  // If the last Parse is newer than FreshTime, we don't check metadata.
+  //   - time_point::min() means we only do IO if we never read the file before
+  //   - time_point::max() means we always at least stat the file
+  //   - steady_clock::now() + seconds(1) means we accept 1 second of staleness
+  void read(const ThreadsafeFS &TFS,
+            std::chrono::steady_clock::time_point FreshTime,
+            llvm::function_ref<void(llvm::Optional<llvm::StringRef>)> Parse,
+            llvm::function_ref<void()> Read) const;
+
+  PathRef path() const { return Path; }
+
+private:
+  std::string Path;
+  // Members are mutable so read() can present a const interface.
+  // (It is threadsafe and approximates read-through to TFS).
+  mutable std::mutex Mu;
+  // Time when the cache was known valid (reflected disk state).
+  mutable std::chrono::steady_clock::time_point ValidTime;
+  // Filesystem metadata corresponding to the currently cached data.
+  mutable std::chrono::system_clock::time_point ModifiedTime;
+  mutable uint64_t Size;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif

clang-tools-extra/clangd/support/FileCache.cpp

+//===--- FileCache.cpp ----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "support/FileCache.h"
+
+namespace clang {
+namespace clangd {
+
+// Sentinel values for the Size cache key. In both cases, a successful stat of
+// the file will never result in the cached value being reused.
+
+// The cached value does not reflect the current content on disk.
+static constexpr uint64_t CacheDiskMismatch =
+    std::numeric_limits<uint64_t>::max();
+// The cached value reflects that the file doesn't exist.
+static constexpr uint64_t FileNotFound = CacheDiskMismatch - 1;
+
+FileCache::FileCache(llvm::StringRef Path)
+    : Path(Path), ValidTime(std::chrono::steady_clock::time_point::min()),
+      ModifiedTime(), Size(CacheDiskMismatch) {
+  assert(llvm::sys::path::is_absolute(Path));
+}
+
+void FileCache::read(
+    const ThreadsafeFS &TFS, std::chrono::steady_clock::time_point FreshTime,
+    llvm::function_ref<void(llvm::Optional<llvm::StringRef>)> Parse,
+    llvm::function_ref<void()> Read) const {
+
+  std::lock_guard<std::mutex> Lock(Mu);
+  // We're going to update the cache and return whatever's in it.
+  auto Return = llvm::make_scope_exit(Read);
+
+  // Return any sufficiently recent result without doing any further work.
+  if (ValidTime > FreshTime)
+    return;
+
+  // Ensure we always bump ValidTime, so that FreshTime imposes a hard limit on
+  // how often we do IO.
+  auto BumpValidTime = llvm::make_scope_exit(
+      [&] { ValidTime = std::chrono::steady_clock::now(); });
+
+  // stat is cheaper than opening the file. It's usually unchanged.
+  assert(llvm::sys::path::is_absolute(Path));
+  auto FS = TFS.view(/*CWD=*/llvm::None);
+  auto Stat = FS->status(Path);
+  if (!Stat || !Stat->isRegularFile()) {
+    if (Size != FileNotFound) // Allow "not found" value to be cached.
+      Parse(llvm::None);
+    // Ensure the cache key won't match any future stat().
+    Size = FileNotFound;
+    return;
+  }
+  // If the modified-time and size match, assume the content does too.
+  if (Size == Stat->getSize() &&
+      ModifiedTime == Stat->getLastModificationTime())
+    return;
+
+  // OK, the file has actually changed. Update cache key, compute new value.
+  Size = Stat->getSize();
+  ModifiedTime = Stat->getLastModificationTime();
+  // Now read the file from disk.
+  if (auto Buf = FS->getBufferForFile(Path)) {
+    Parse(Buf->get()->getBuffer());
+    // Result is cacheable if the actual read size matches the new cache key.
+    // (We can't update the cache key, because we don't know the new mtime).
+    if (Buf->get()->getBufferSize() != Size)
+      Size = CacheDiskMismatch;
+  } else {
+    // File was unreadable. Keep the old value and try again next time.
+    Size = CacheDiskMismatch;
+  }
+}
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

TEST(ProviderTest, FromAncestorRelativeYAMLFiles) {
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
 
   FS.Files.erase("a/foo.yaml");
   Cfg = P->getConfig(ABCParams, Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
 }
 
+// FIXME: delete this test, it's covered by FileCacheTests.
 TEST(ProviderTest, Staleness) {
   MockFS FS;
 
   auto StartTime = std::chrono::steady_clock::now();
   Params StaleOK;
   StaleOK.FreshTime = StartTime;
   Params MustBeFresh;
   MustBeFresh.FreshTime = StartTime + std::chrono::hours(1);

clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp

+//===-- FileCacheTests.cpp ------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "support/FileCache.h"
+
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <atomic>
+#include <chrono>
+
+namespace clang {
+namespace clangd {
+namespace config {
+namespace {
+
+class TestCache : public FileCache {
+  MockFS FS;
+  mutable std::string Value;
+
+public:
+  TestCache() : FileCache(testPath("foo.cc")) {}
+
+  void setContents(const char *C) {
+    if (C)
+      FS.Files[testPath("foo.cc")] = C;
+    else
+      FS.Files.erase(testPath("foo.cc"));
+  }
+
+  std::string get(std::chrono::steady_clock::time_point FreshTime,
+                  bool ExpectParse) const {
+    bool GotParse = false;
+    bool GotRead;
+    std::string Result;
+    read(
+        FS, FreshTime,
+        [&](llvm::Optional<llvm::StringRef> Data) {
+          GotParse = true;
+          Value = Data.getValueOr("").str();
+        },
+        [&]() {
+          GotRead = true;
+          Result = Value;
+        });
+    EXPECT_EQ(GotParse, ExpectParse);
+    EXPECT_TRUE(GotRead);
+    return Result;
+  }
+};
+
+TEST(FileCacheTest, Invalidation) {
+  TestCache C;
+
+  auto StaleOK = std::chrono::steady_clock::now();
+  auto MustBeFresh = StaleOK + std::chrono::hours(1);
+
+  C.setContents("a");
+  EXPECT_EQ("a", C.get(StaleOK, /*ExpectParse=*/true)) << "Parsed first time";
+  EXPECT_EQ("a", C.get(StaleOK, /*ExpectParse=*/false)) << "Cached (time)";
+  EXPECT_EQ("a", C.get(MustBeFresh, /*ExpectParse=*/false)) << "Cached (stat)";
+  C.setContents("bb");
+  EXPECT_EQ("a", C.get(StaleOK, /*ExpectParse=*/false)) << "Cached (time)";
+  EXPECT_EQ("bb", C.get(MustBeFresh, /*ExpectParse=*/true)) << "Size changed";
+  EXPECT_EQ("bb", C.get(MustBeFresh, /*ExpectParse=*/true)) << "Cached (stat)";
+  C.setContents(nullptr);
+  EXPECT_EQ("bb", C.get(StaleOK, /*ExpectParse=*/false)) << "Cached (time)";
+  EXPECT_EQ("", C.get(MustBeFresh, /*ExpectParse=*/true)) << "Stat failed";
+  EXPECT_EQ("", C.get(MustBeFresh, /*ExpectParse=*/false)) << "Cached (404)";
+  C.setContents("bb"); // Match the previous stat values!
+  EXPECT_EQ("", C.get(StaleOK, /*ExpectParse=*/false)) << "Cached (time)";
+  EXPECT_EQ("bb", C.get(MustBeFresh, /*ExpectParse=*/true)) << "Size changed";
+}
+
+} // namespace
+} // namespace config
+} // namespace clangd
+} // namespace clang