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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits