ilya-biryukov updated this revision to Diff 150920.
ilya-biryukov added a comment.
- Revert "Remove CachingCompilationDb". Turns out we do need it internally
(Thanks, Sam!) :-)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48068
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CompileArgsCache.cpp
clangd/CompileArgsCache.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
unittests/clangd/ClangdTests.cpp
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
Index: unittests/clangd/SyncAPI.h
===================================================================
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -20,8 +20,7 @@
// Calls addDocument and then blockUntilIdleForTest.
void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents,
- WantDiagnostics WantDiags = WantDiagnostics::Auto,
- bool SkipCache = false);
+ WantDiagnostics WantDiags = WantDiagnostics::Auto);
llvm::Expected<CompletionList>
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
Index: unittests/clangd/SyncAPI.cpp
===================================================================
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -12,8 +12,8 @@
namespace clangd {
void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents,
- WantDiagnostics WantDiags, bool SkipCache) {
- Server.addDocument(File, Contents, WantDiags, SkipCache);
+ WantDiagnostics WantDiags) {
+ Server.addDocument(File, Contents, WantDiags);
if (!Server.blockUntilIdleForTest())
llvm_unreachable("not idle after addDocument");
}
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -390,16 +390,7 @@
// Now switch to C++ mode.
CDB.ExtraClangFlags = {"-xc++"};
- // By default addDocument does not check if CompileCommand has changed, so we
- // expect to see the errors.
- runAddDocument(Server, FooCpp, SourceContents1);
- EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
- runAddDocument(Server, FooCpp, SourceContents2);
- EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
- // Passing SkipCache=true will force addDocument to reparse the file with
- // proper flags.
- runAddDocument(Server, FooCpp, SourceContents2, WantDiagnostics::Auto,
- /*SkipCache=*/true);
+ runAddDocument(Server, FooCpp, SourceContents2, WantDiagnostics::Auto);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
// Subsequent addDocument calls should finish without errors too.
runAddDocument(Server, FooCpp, SourceContents1);
@@ -431,14 +422,7 @@
// Parse without the define, no errors should be produced.
CDB.ExtraClangFlags = {};
- // By default addDocument does not check if CompileCommand has changed, so we
- // expect to see the errors.
- runAddDocument(Server, FooCpp, SourceContents);
- EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
- // Passing SkipCache=true will force addDocument to reparse the file with
- // proper flags.
- runAddDocument(Server, FooCpp, SourceContents, WantDiagnostics::Auto,
- /*SkipCache=*/true);
+ runAddDocument(Server, FooCpp, SourceContents, WantDiagnostics::Auto);
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
// Subsequent addDocument call should finish without errors too.
@@ -500,10 +484,8 @@
CDB.ExtraClangFlags.clear();
DiagConsumer.clear();
Server.removeDocument(BazCpp);
- Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto,
- /*SkipCache=*/true);
- Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto,
- /*SkipCache=*/true);
+ Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto);
+ Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto);
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_THAT(DiagConsumer.filesWithDiags(),
@@ -708,7 +690,7 @@
Server.addDocument(FilePaths[FileIndex],
ShouldHaveErrors ? SourceContentsWithErrors
: SourceContentsWithoutErrors,
- WantDiagnostics::Auto, SkipCache);
+ WantDiagnostics::Auto);
UpdateStatsOnAddDocument(FileIndex, ShouldHaveErrors);
};
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -85,6 +85,34 @@
/// is located.
llvm::Optional<Path> CompileCommandsDir;
};
+
+/// A wrapper around GlobalCompilationDatabase that caches the compile commands.
+/// Note that only results of getCompileCommand are cached.
+class CachingCompilationDb : public GlobalCompilationDatabase {
+public:
+ explicit CachingCompilationDb(GlobalCompilationDatabase &InnerCDB);
+
+ /// Gets compile command for \p File from cache or CDB if it's not in the
+ /// cache.
+ llvm::Optional<tooling::CompileCommand>
+ getCompileCommand(PathRef File) const override;
+
+ /// Forwards to the inner CDB. Results of this function are not cached.
+ tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+
+ /// Removes an entry for \p File if it's present in the cache.
+ void invalidate(PathRef File);
+
+ /// Removes all cached compile commands.
+ void clear();
+
+private:
+ GlobalCompilationDatabase &InnerCDB;
+ mutable std::mutex Mut;
+ mutable llvm::StringMap<llvm::Optional<tooling::CompileCommand>>
+ Cached; /* GUARDED_BY(Mut) */
+};
+
} // namespace clangd
} // namespace clang
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -119,5 +119,42 @@
return nullptr;
}
+CachingCompilationDb::CachingCompilationDb(GlobalCompilationDatabase &InnerCDB)
+ : InnerCDB(InnerCDB) {}
+
+llvm::Optional<tooling::CompileCommand>
+CachingCompilationDb::getCompileCommand(PathRef File) const {
+ std::unique_lock<std::mutex> Lock(Mut);
+ auto It = Cached.find(File);
+ if (It != Cached.end())
+ return It->second;
+
+ Lock.unlock();
+ llvm::Optional<tooling::CompileCommand> Command =
+ InnerCDB.getCompileCommand(File);
+ Lock.lock();
+ // 'It' may be invalid at this point, recompute it.
+ It = Cached.find(File);
+ if (It != Cached.end())
+ return It->second;
+ Cached.insert({File, Command});
+ return Command;
+}
+
+tooling::CompileCommand
+CachingCompilationDb::getFallbackCommand(PathRef File) const {
+ return InnerCDB.getFallbackCommand(File);
+}
+
+void CachingCompilationDb::invalidate(PathRef File) {
+ std::unique_lock<std::mutex> Lock(Mut);
+ Cached.erase(File);
+}
+
+void CachingCompilationDb::clear() {
+ std::unique_lock<std::mutex> Lock(Mut);
+ Cached.clear();
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/CompileArgsCache.h
===================================================================
--- clangd/CompileArgsCache.h
+++ /dev/null
@@ -1,43 +0,0 @@
-//===--- CompileArgsCache.h -------------------------------------*- C++-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===---------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILEARGSCACHE_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILEARGSCACHE_H
-
-#include "GlobalCompilationDatabase.h"
-#include "Path.h"
-#include "clang/Tooling/CompilationDatabase.h"
-
-namespace clang {
-namespace clangd {
-
-/// A helper class used by ClangdServer to get compile commands from CDB.
-/// Also caches CompileCommands produced by compilation database on per-file
-/// basis. This avoids queries to CDB that can be much more expensive than a
-/// table lookup.
-class CompileArgsCache {
-public:
- CompileArgsCache(GlobalCompilationDatabase &CDB, Path ResourceDir);
-
- /// Gets compile command for \p File from cache or CDB if it's not in the
- /// cache.
- tooling::CompileCommand getCompileCommand(PathRef File);
-
- /// Removes a cache entry for \p File, if it's present in the cache.
- void invalidate(PathRef File);
-
-private:
- GlobalCompilationDatabase &CDB;
- const Path ResourceDir;
- llvm::StringMap<tooling::CompileCommand> Cached;
-};
-
-} // namespace clangd
-} // namespace clang
-#endif
Index: clangd/CompileArgsCache.cpp
===================================================================
--- clangd/CompileArgsCache.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-//===--- CompileArgsCache.cpp --------------------------------------------===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===---------------------------------------------------------------------===//
-
-#include "CompileArgsCache.h"
-
-namespace clang {
-namespace clangd {
-namespace {
-tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB,
- PathRef File, PathRef ResourceDir) {
- llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
- if (!C) // FIXME: Suppress diagnostics? Let the user know?
- C = CDB.getFallbackCommand(File);
-
- // Inject the resource dir.
- // FIXME: Don't overwrite it if it's already there.
- C->CommandLine.push_back("-resource-dir=" + ResourceDir.str());
- return std::move(*C);
-}
-} // namespace
-
-CompileArgsCache::CompileArgsCache(GlobalCompilationDatabase &CDB,
- Path ResourceDir)
- : CDB(CDB), ResourceDir(std::move(ResourceDir)) {}
-
-tooling::CompileCommand CompileArgsCache::getCompileCommand(PathRef File) {
- auto It = Cached.find(File);
- if (It == Cached.end()) {
- It = Cached.insert({File, clangd::getCompileCommand(CDB, File, ResourceDir)})
- .first;
- }
- return It->second;
-}
-
-void CompileArgsCache::invalidate(PathRef File) { Cached.erase(File); }
-
-} // namespace clangd
-} // namespace clang
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -12,7 +12,6 @@
#include "ClangdUnit.h"
#include "CodeComplete.h"
-#include "CompileArgsCache.h"
#include "Function.h"
#include "GlobalCompilationDatabase.h"
#include "Protocol.h"
@@ -122,8 +121,7 @@
/// When \p SkipCache is true, compile commands will always be requested from
/// compilation database even if they were cached in previous invocations.
void addDocument(PathRef File, StringRef Contents,
- WantDiagnostics WD = WantDiagnostics::Auto,
- bool SkipCache = false);
+ WantDiagnostics WD = WantDiagnostics::Auto);
/// Remove \p File from list of tracked files, schedule a request to free
/// resources associated with it.
@@ -216,13 +214,16 @@
void consumeDiagnostics(PathRef File, DocVersion Version,
std::vector<Diag> Diags);
- CompileArgsCache CompileArgs;
+ tooling::CompileCommand getCompileCommand(PathRef File);
+
+ GlobalCompilationDatabase &CDB;
DiagnosticsConsumer &DiagConsumer;
FileSystemProvider &FSProvider;
/// Used to synchronize diagnostic responses for added and removed files.
llvm::StringMap<DocVersion> InternalVersion;
+ Path ResourceDir;
// The index used to look up symbols. This could be:
// - null (all index functionality is optional)
// - the dynamic index owned by ClangdServer (FileIdx)
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -23,6 +23,7 @@
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -83,9 +84,9 @@
FileSystemProvider &FSProvider,
DiagnosticsConsumer &DiagConsumer,
const Options &Opts)
- : CompileArgs(CDB, Opts.ResourceDir ? Opts.ResourceDir->str()
- : getStandardResourceDir()),
- DiagConsumer(DiagConsumer), FSProvider(FSProvider),
+ : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
+ ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
+ : getStandardResourceDir()),
FileIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
PCHs(std::make_shared<PCHContainerOperations>()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
@@ -120,13 +121,10 @@
}
void ClangdServer::addDocument(PathRef File, StringRef Contents,
- WantDiagnostics WantDiags, bool SkipCache) {
- if (SkipCache)
- CompileArgs.invalidate(File);
-
+ WantDiagnostics WantDiags) {
DocVersion Version = ++InternalVersion[File];
- ParseInputs Inputs = {CompileArgs.getCompileCommand(File),
- FSProvider.getFileSystem(), Contents.str()};
+ ParseInputs Inputs = {getCompileCommand(File), FSProvider.getFileSystem(),
+ Contents.str()};
Path FileStr = File.str();
WorkScheduler.update(File, std::move(Inputs), WantDiags,
@@ -137,7 +135,6 @@
void ClangdServer::removeDocument(PathRef File) {
++InternalVersion[File];
- CompileArgs.invalidate(File);
WorkScheduler.remove(File);
}
@@ -430,6 +427,17 @@
DiagConsumer.onDiagnosticsReady(File, std::move(Diags));
}
+tooling::CompileCommand ClangdServer::getCompileCommand(PathRef File) {
+ llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
+ if (!C) // FIXME: Suppress diagnostics? Let the user know?
+ C = CDB.getFallbackCommand(File);
+
+ // Inject the resource dir.
+ // FIXME: Don't overwrite it if it's already there.
+ C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+ return std::move(*C);
+}
+
void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
// FIXME: Do nothing for now. This will be used for indexing and potentially
// invalidating other caches.
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -100,7 +100,9 @@
// Various ClangdServer parameters go here. It's important they're created
// before ClangdServer.
- DirectoryBasedGlobalCompilationDatabase CDB;
+ DirectoryBasedGlobalCompilationDatabase NonCachedCDB;
+ CachingCompilationDb CDB;
+
RealFileSystemProvider FSProvider;
/// Options used for code completion
clangd::CodeCompleteOptions CCOpts;
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -129,11 +129,13 @@
void ClangdLSPServer::onExit(ExitParams &Params) { IsDone = true; }
void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
- if (Params.metadata && !Params.metadata->extraFlags.empty())
- CDB.setExtraFlagsForFile(Params.textDocument.uri.file(),
- std::move(Params.metadata->extraFlags));
-
PathRef File = Params.textDocument.uri.file();
+ if (Params.metadata && !Params.metadata->extraFlags.empty()) {
+ NonCachedCDB.setExtraFlagsForFile(File,
+ std::move(Params.metadata->extraFlags));
+ CDB.invalidate(File);
+ }
+
std::string &Contents = Params.textDocument.text;
DraftMgr.addDraft(File, Contents);
@@ -155,6 +157,7 @@
// fail rather than giving wrong results.
DraftMgr.removeDraft(File);
Server.removeDocument(File);
+ CDB.invalidate(File);
log(llvm::toString(Contents.takeError()));
return;
}
@@ -383,17 +386,20 @@
// Compilation database change.
if (Settings.compilationDatabasePath.hasValue()) {
- CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
+ NonCachedCDB.setCompileCommandsDir(
+ Settings.compilationDatabasePath.getValue());
+ CDB.clear();
+
reparseOpenedFiles();
}
}
ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
const clangd::CodeCompleteOptions &CCOpts,
llvm::Optional<Path> CompileCommandsDir,
const ClangdServer::Options &Opts)
- : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts),
- SupportedSymbolKinds(defaultSymbolKinds()),
+ : Out(Out), NonCachedCDB(std::move(CompileCommandsDir)), CDB(NonCachedCDB),
+ CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
Server(CDB, FSProvider, /*DiagConsumer=*/*this, Opts) {}
bool ClangdLSPServer::run(std::FILE *In, JSONStreamStyle InputStyle) {
@@ -471,6 +477,5 @@
void ClangdLSPServer::reparseOpenedFiles() {
for (const Path &FilePath : DraftMgr.getActiveFiles())
Server.addDocument(FilePath, *DraftMgr.getDraft(FilePath),
- WantDiagnostics::Auto,
- /*SkipCache=*/true);
+ WantDiagnostics::Auto);
}
Index: clangd/CMakeLists.txt
===================================================================
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -14,7 +14,6 @@
ClangdUnit.cpp
CodeComplete.cpp
CodeCompletionStrings.cpp
- CompileArgsCache.cpp
Compiler.cpp
Context.cpp
Diagnostics.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
