maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Some nitpicking around the new split (preamble/main) dynamic index 

Summary:
- DynamicIndex doesn't implement ParsingCallbacks, to make its role clearer.
  ParsingCallbacks is a separate object owned by the receiving TUScheduler.
  (I tried to get rid of the "index-like-object that doesn't implement index"
  but it was too messy).
- Clarified(?) docs around DynamicIndex - fewer details up front, more details
  inside.
- Exposed dynamic index from ClangdServer for memory monitoring and more
  direct testing of its contents (actual tests not added here, wanted to get
  this out for review)
- Removed a redundant and sligthly confusing filename param in a callback

Reviewers: ilya-biryukov

Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D51221

llvm-svn: 341325
This commit is contained in:
Sam McCall 2018-09-03 16:37:59 +00:00
parent adc178ef2c
commit 046557bc03
10 changed files with 88 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
clang-tools-extra/clangd/ClangdServer.cpp
View File

@ -71,34 +71,57 @@ public:
};
} // namespace
/// Manages dynamic index for open files. Each file might contribute two sets
/// of symbols to the dynamic index: symbols from the preamble and symbols
/// from the file itself. Those have different lifetimes and we merge results
/// from both
class ClangdServer::DynamicIndex : public ParsingCallbacks {
/// The dynamic index tracks symbols visible in open files.
/// For boring reasons, it doesn't implement SymbolIndex directly - use index().
class ClangdServer::DynamicIndex {
public:
DynamicIndex(std::vector<std::string> URISchemes)
: PreambleIdx(URISchemes), MainFileIdx(URISchemes),
MergedIndex(mergeIndex(&MainFileIdx, &PreambleIdx)) {}
SymbolIndex &index() const { return *MergedIndex; }
const SymbolIndex &index() const { return *MergedIndex; }
void onPreambleAST(PathRef Path, ASTContext &Ctx,
std::shared_ptr<clang::Preprocessor> PP) override {
PreambleIdx.update(Path, &Ctx, PP, /*TopLevelDecls=*/llvm::None);
}
// Returns callbacks that can be used to update the index with new ASTs.
// Index() presents a merged view of the supplied main-file and preamble ASTs.
std::unique_ptr<ParsingCallbacks> makeUpdateCallbacks() {
struct CB : public ParsingCallbacks {
CB(ClangdServer::DynamicIndex *This) : This(This) {}
DynamicIndex *This;
void onMainAST(PathRef Path, ParsedAST &AST) override {
void onPreambleAST(PathRef Path, ASTContext &Ctx,
std::shared_ptr<clang::Preprocessor> PP) override {
This->PreambleIdx.update(Path, &Ctx, std::move(PP));
}
MainFileIdx.update(Path, &AST.getASTContext(), AST.getPreprocessorPtr(),
AST.getLocalTopLevelDecls());
}
void onMainAST(PathRef Path, ParsedAST &AST) override {
This->MainFileIdx.update(Path, &AST.getASTContext(),
AST.getPreprocessorPtr(),
AST.getLocalTopLevelDecls());
}
};
return llvm::make_unique<CB>(this);
};
private:
// Contains information from each file's preamble only.
// These are large, but update fairly infrequently (preambles are stable).
// Missing information:
// - symbol occurrences (these are always "from the main file")
// - definition locations in the main file
//
// FIXME: Because the preambles for different TUs have large overlap and
// FileIndex doesn't deduplicate, this uses lots of extra RAM.
// The biggest obstacle in fixing this: the obvious approach of partitioning
// by declaring file (rather than main file) fails if headers provide
// different symbols based on preprocessor state.
FileIndex PreambleIdx;
// Contains information from each file's main AST.
// These are updated frequently (on file change), but are relatively small.
// Mostly contains:
// - occurrences of symbols declared in the preamble and referenced from main
// - symbols declared both in the main file and the preamble
// (Note that symbols *only* in the main file are not indexed).
FileIndex MainFileIdx;
/// Merged view into both indexes. Merges are performed in a similar manner
/// to the merges of dynamic and static index.
std::unique_ptr<SymbolIndex> MergedIndex;
};
@ -127,7 +150,7 @@ ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
// FIXME(ioeric): this can be slow and we may be able to index on less
// critical paths.
WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
DynamicIdx ? *DynamicIdx : noopParsingCallbacks(),
DynamicIdx ? DynamicIdx->makeUpdateCallbacks() : nullptr,
Opts.UpdateDebounce, Opts.RetentionPolicy) {
if (DynamicIdx && Opts.StaticIndex) {
MergedIndex = mergeIndex(&DynamicIdx->index(), Opts.StaticIndex);
@ -144,6 +167,10 @@ ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
// ClangdServer::DynamicIndex.
ClangdServer::~ClangdServer() = default;
const SymbolIndex *ClangdServer::dynamicIndex() const {
return DynamicIdx ? &DynamicIdx->index() : nullptr;
}
void ClangdServer::setRootPath(PathRef RootPath) {
auto FS = FSProvider.getFileSystem();
auto Status = FS->status(RootPath);

11
clang-tools-extra/clangd/ClangdServer.h
View File

@ -191,6 +191,10 @@ public:
/// FIXME: those metrics might be useful too, we should add them.
std::vector<std::pair<Path, std::size_t>> getUsedBytesPerFile() const;
/// Returns the active dynamic index if one was built.
/// This can be useful for testing, debugging, or observing memory usage.
const SymbolIndex *dynamicIndex() const;
// Blocks the main thread until the server is idle. Only for use in tests.
// Returns false if the timeout expires.
LLVM_NODISCARD bool
@ -224,11 +228,10 @@ private:
// - the dynamic index owned by ClangdServer (DynamicIdx)
// - the static index passed to the constructor
// - a merged view of a static and dynamic index (MergedIndex)
SymbolIndex *Index;
/// If present, an up-to-date of symbols in open files. Read via Index.
const SymbolIndex *Index;
// If present, an index of symbols in open files. Read via *Index.
std::unique_ptr<DynamicIndex> DynamicIdx;
// If present, a merged view of DynamicIdx and an external index. Read via
// Index.
// If present, storage for the merged static/dynamic index. Read via *Index.
std::unique_ptr<SymbolIndex> MergedIndex;
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)

2
clang-tools-extra/clangd/ClangdUnit.cpp
View File

@ -95,7 +95,7 @@ public:
if (!ParsedCallback)
return;
trace::Span Tracer("Running PreambleCallback");
ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr());
}
void BeforeExecute(CompilerInstance &CI) override {

4
clang-tools-extra/clangd/ClangdUnit.h
View File

@ -127,8 +127,8 @@ private:
IncludeStructure Includes;
};
using PreambleParsedCallback = std::function<void(
PathRef Path, ASTContext &, std::shared_ptr<clang::Preprocessor>)>;
using PreambleParsedCallback =
std::function<void(ASTContext &, std::shared_ptr<clang::Preprocessor>)>;
/// Builds compiler invocation that could be used to build AST or preamble.
std::unique_ptr<CompilerInvocation>

4
clang-tools-extra/clangd/CodeComplete.cpp
View File

@ -798,7 +798,7 @@ struct ScoredSignature {
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
SymbolIndex *Index, SignatureHelp &SigHelp)
const SymbolIndex *Index, SignatureHelp &SigHelp)
: CodeCompleteConsumer(CodeCompleteOpts,
/*OutputIsBinary=*/false),
SigHelp(SigHelp),
@ -1584,7 +1584,7 @@ SignatureHelp signatureHelp(PathRef FileName,
StringRef Contents, Position Pos,
IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs,
SymbolIndex *Index) {
const SymbolIndex *Index) {
SignatureHelp Result;
clang::CodeCompleteOptions Options;
Options.IncludeGlobals = false;

12
clang-tools-extra/clangd/CodeComplete.h
View File

@ -220,11 +220,13 @@ CodeCompleteResult codeComplete(PathRef FileName,
SpeculativeFuzzyFind *SpecFuzzyFind = nullptr);
/// Get signature help at a specified \p Pos in \p FileName.
SignatureHelp
signatureHelp(PathRef FileName, const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble, StringRef Contents,
Position Pos, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs, SymbolIndex *Index);
SignatureHelp signatureHelp(PathRef FileName,
const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble,
StringRef Contents, Position Pos,
IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs,
const SymbolIndex *Index);
// For index-based completion, we only consider:
// * symbols in namespaces or translation unit scopes (e.g. no class

26
clang-tools-extra/clangd/TUScheduler.cpp
View File

@ -365,13 +365,12 @@ void ASTWorker::update(
std::shared_ptr<const PreambleData> OldPreamble =
getPossiblyStalePreamble();
std::shared_ptr<const PreambleData> NewPreamble =
buildPreamble(FileName, *Invocation, OldPreamble, OldCommand, Inputs,
PCHs, StorePreambleInMemory,
[this](PathRef Path, ASTContext &Ctx,
std::shared_ptr<clang::Preprocessor> PP) {
Callbacks.onPreambleAST(FileName, Ctx, std::move(PP));
});
std::shared_ptr<const PreambleData> NewPreamble = buildPreamble(
FileName, *Invocation, OldPreamble, OldCommand, Inputs, PCHs,
StorePreambleInMemory,
[this](ASTContext &Ctx, std::shared_ptr<clang::Preprocessor> PP) {
Callbacks.onPreambleAST(FileName, Ctx, std::move(PP));
});
bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
{
@ -654,11 +653,6 @@ unsigned getDefaultAsyncThreadsCount() {
return HardwareConcurrency;
}
ParsingCallbacks &noopParsingCallbacks() {
static ParsingCallbacks *Instance = new ParsingCallbacks;
return *Instance;
}
struct TUScheduler::FileData {
/// Latest inputs, passed to TUScheduler::update().
std::string Contents;
@ -668,11 +662,13 @@ struct TUScheduler::FileData {
TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
ParsingCallbacks &Callbacks,
std::unique_ptr<ParsingCallbacks> Callbacks,
std::chrono::steady_clock::duration UpdateDebounce,
ASTRetentionPolicy RetentionPolicy)
: StorePreamblesInMemory(StorePreamblesInMemory),
PCHOps(std::make_shared<PCHContainerOperations>()), Callbacks(Callbacks),
PCHOps(std::make_shared<PCHContainerOperations>()),
Callbacks(Callbacks ? move(Callbacks)
: llvm::make_unique<ParsingCallbacks>()),
Barrier(AsyncThreadsCount),
IdleASTs(llvm::make_unique<ASTCache>(RetentionPolicy.MaxRetainedASTs)),
UpdateDebounce(UpdateDebounce) {
@ -711,7 +707,7 @@ void TUScheduler::update(
// Create a new worker to process the AST-related tasks.
ASTWorkerHandle Worker = ASTWorker::create(
File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, Callbacks);
Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, *Callbacks);
FD = std::unique_ptr<FileData>(new FileData{
Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
} else {

6
clang-tools-extra/clangd/TUScheduler.h
View File

@ -74,8 +74,6 @@ public:
virtual void onMainAST(PathRef Path, ParsedAST &AST) {}
};
ParsingCallbacks &noopParsingCallbacks();
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@ -86,7 +84,7 @@ ParsingCallbacks &noopParsingCallbacks();
class TUScheduler {
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
ParsingCallbacks &ASTCallbacks,
std::unique_ptr<ParsingCallbacks> ASTCallbacks,
std::chrono::steady_clock::duration UpdateDebounce,
ASTRetentionPolicy RetentionPolicy);
~TUScheduler();
@ -166,7 +164,7 @@ public:
private:
const bool StorePreamblesInMemory;
const std::shared_ptr<PCHContainerOperations> PCHOps;
ParsingCallbacks &Callbacks;
std::unique_ptr<ParsingCallbacks> Callbacks; // not nullptr
Semaphore Barrier;
llvm::StringMap<std::unique_ptr<FileData>> Files;
std::unique_ptr<ASTCache> IdleASTs;

5
clang-tools-extra/unittests/clangd/FileIndexTests.cpp
View File

@ -296,11 +296,10 @@ TEST(FileIndexTest, RebuildWithPreamble) {
buildPreamble(
FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
std::make_shared<PCHContainerOperations>(), /*StoreInMemory=*/true,
[&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
std::shared_ptr<Preprocessor> PP) {
[&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP) {
EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
IndexUpdated = true;
Index.update(FilePath, &Ctx, std::move(PP));
Index.update(FooCpp, &Ctx, std::move(PP));
});
ASSERT_TRUE(IndexUpdated);

20
clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp
View File

@ -46,7 +46,7 @@ protected:
TEST_F(TUSchedulerTests, MissingFiles) {
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@ -104,7 +104,7 @@ TEST_F(TUSchedulerTests, WantDiagnostics) {
Notification Ready;
TUScheduler S(
getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
auto Path = testPath("foo.cpp");
@ -132,7 +132,7 @@ TEST_F(TUSchedulerTests, Debounce) {
std::atomic<int> CallbackCount(0);
{
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::seconds(1),
ASTRetentionPolicy());
// FIXME: we could probably use timeouts lower than 1 second here.
@ -165,7 +165,7 @@ TEST_F(TUSchedulerTests, PreambleConsistency) {
Notification InconsistentReadDone; // Must live longest.
TUScheduler S(
getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
noopParsingCallbacks(),
/*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
auto Path = testPath("foo.cpp");
@ -221,7 +221,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
// Run TUScheduler and collect some stats.
{
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::milliseconds(50),
ASTRetentionPolicy());
@ -319,7 +319,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
Policy.MaxRetainedASTs = 2;
TUScheduler S(
/*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
noopParsingCallbacks(),
/*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
llvm::StringLiteral SourceContents = R"cpp(
@ -369,7 +369,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
TEST_F(TUSchedulerTests, EmptyPreamble) {
TUScheduler S(
/*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
noopParsingCallbacks(),
/*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@ -416,7 +416,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
// the same time. All reads should get the same non-null preamble.
TUScheduler S(
/*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
noopParsingCallbacks(),
/*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
auto Foo = testPath("foo.cpp");
@ -449,7 +449,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
TUScheduler S(
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
/*StorePreambleInMemory=*/true, noopParsingCallbacks(),
/*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@ -502,7 +502,7 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
TEST_F(TUSchedulerTests, NoChangeDiags) {
TUScheduler S(
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
/*StorePreambleInMemory=*/true, noopParsingCallbacks(),
/*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());