[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

As discussed offline, the cache is always missing today, but we do have reason to believe we're doing a fair amount of IO in `buildCompilerInvocation` and it should be very cacheable.
So dropping the cache here might be the wrong direction vs fixing it.
I don't know:

- for sure how much IO is there or what it is
- whether it's mostly stats and so could be handled by this mechanism
- exactly what scope we should be filling the cache at (it's really tempting to do it on startup, which is a bigger change)

If the plan is to make the statcache wrap providers instead of/as well as FSes, do we actually need this change? (Then we can avoid having to decide right now)



================
Comment at: clang-tools-extra/clangd/Preamble.cpp:215
              const tooling::CompileCommand &Cmd) {
-  // FIXME: Change PreambleStatCache to operate on FileSystemProvider rather
-  // than vfs::FileSystem, that way we can just use ParseInputs without this
-  // hack.
-  auto GetFSProvider = [](llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
-    class VFSProvider : public FileSystemProvider {
+  auto EmptyFSProvider = [] {
+    class EmptyProvider : public FileSystemProvider {
----------------
This is confusing - this class is essentially unused (move to next patch?) and erasing the class with a lambda seems unnecessarily obscure.


================
Comment at: clang-tools-extra/clangd/Preamble.cpp:248
       // also implies missing resolved paths for includes.
-      new llvm::vfs::InMemoryFileSystem, IgnoreDiags);
+      EmptyFSProvider()->getFileSystem(), IgnoreDiags);
   if (Clang->getFrontendOpts().Inputs.empty())
----------------
(This change isn't related to the description, which threw me for a loop - may want to defer it)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81719/new/

https://reviews.llvm.org/D81719