[clang-tools-extra] r344594 - [clangd] Optionally use dex for the preamble parts of the dynamic index.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 16 01:53:53 PDT 2018
Author: sammccall
Date: Tue Oct 16 01:53:52 2018
New Revision: 344594
URL: http://llvm.org/viewvc/llvm-project?rev=344594&view=rev
Log:
[clangd] Optionally use dex for the preamble parts of the dynamic index.
Summary:
Reuse the old -use-dex-index experiment flag for this.
To avoid breaking the tests, make Dex deduplicate symbols, addressing an old FIXME.
Reviewers: hokein
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53288
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/clangd/index/dex/Dex.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Oct 16 01:53:52 2018
@@ -105,8 +105,10 @@ ClangdServer::ClangdServer(const GlobalC
: CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
: getStandardResourceDir()),
- DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex(Opts.URISchemes)
- : nullptr),
+ DynamicIdx(Opts.BuildDynamicSymbolIndex
+ ? new FileIndex(Opts.URISchemes,
+ Opts.HeavyweightDynamicSymbolIndex)
+ : nullptr),
PCHs(std::make_shared<PCHContainerOperations>()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Oct 16 01:53:52 2018
@@ -75,6 +75,9 @@ public:
/// If true, ClangdServer builds a dynamic in-memory index for symbols in
/// opened files and uses the index to augment code completion results.
bool BuildDynamicSymbolIndex = false;
+ /// Use a heavier and faster in-memory index implementation.
+ /// FIXME: we should make this true if it isn't too slow to build!.
+ bool HeavyweightDynamicSymbolIndex = false;
/// URI schemes to use when building the dynamic index.
/// If empty, the default schemes in SymbolCollector will be used.
Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Oct 16 01:53:52 2018
@@ -183,7 +183,7 @@ llvm::Error BackgroundIndex::index(tooli
// FIXME: this should rebuild once-in-a-while, not after every file.
// At that point we should use Dex, too.
vlog("Rebuilding automatic index");
- reset(IndexedSymbols.buildMemIndex());
+ reset(IndexedSymbols.buildIndex(IndexType::Light));
return Error::success();
}
Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Tue Oct 16 01:53:52 2018
@@ -13,6 +13,7 @@
#include "SymbolCollector.h"
#include "index/Index.h"
#include "index/Merge.h"
+#include "index/dex/Dex.h"
#include "clang/Index/IndexingAction.h"
#include "clang/Lex/MacroInfo.h"
#include "clang/Lex/Preprocessor.h"
@@ -98,7 +99,8 @@ void FileSymbols::update(PathRef Path, s
FileToRefs[Path] = std::move(Refs);
}
-std::unique_ptr<SymbolIndex> FileSymbols::buildMemIndex() {
+std::unique_ptr<SymbolIndex>
+FileSymbols::buildIndex(IndexType Type, ArrayRef<std::string> URISchemes) {
std::vector<std::shared_ptr<SymbolSlab>> SymbolSlabs;
std::vector<std::shared_ptr<RefSlab>> RefSlabs;
{
@@ -144,18 +146,27 @@ std::unique_ptr<SymbolIndex> FileSymbols
StorageSize += RefSlab->bytes();
// Index must keep the slabs and contiguous ranges alive.
- return llvm::make_unique<MemIndex>(
- llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
- std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
- std::move(RefsStorage)),
- StorageSize);
+ switch (Type) {
+ case IndexType::Light:
+ return llvm::make_unique<MemIndex>(
+ llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
+ std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
+ std::move(RefsStorage)),
+ StorageSize);
+ case IndexType::Heavy:
+ return llvm::make_unique<dex::Dex>(
+ llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
+ std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
+ std::move(RefsStorage)),
+ StorageSize, std::move(URISchemes));
+ }
}
-FileIndex::FileIndex(std::vector<std::string> URISchemes)
- : MergedIndex(&MainFileIndex, &PreambleIndex),
+FileIndex::FileIndex(std::vector<std::string> URISchemes, bool UseDex)
+ : MergedIndex(&MainFileIndex, &PreambleIndex), UseDex(UseDex),
URISchemes(std::move(URISchemes)),
- PreambleIndex(PreambleSymbols.buildMemIndex()),
- MainFileIndex(MainFileSymbols.buildMemIndex()) {}
+ PreambleIndex(llvm::make_unique<MemIndex>()),
+ MainFileIndex(llvm::make_unique<MemIndex>()) {}
void FileIndex::updatePreamble(PathRef Path, ASTContext &AST,
std::shared_ptr<Preprocessor> PP) {
@@ -163,7 +174,8 @@ void FileIndex::updatePreamble(PathRef P
PreambleSymbols.update(Path,
llvm::make_unique<SymbolSlab>(std::move(Symbols)),
llvm::make_unique<RefSlab>());
- PreambleIndex.reset(PreambleSymbols.buildMemIndex());
+ PreambleIndex.reset(PreambleSymbols.buildIndex(
+ UseDex ? IndexType::Heavy : IndexType::Light, URISchemes));
}
void FileIndex::updateMain(PathRef Path, ParsedAST &AST) {
@@ -171,7 +183,7 @@ void FileIndex::updateMain(PathRef Path,
MainFileSymbols.update(
Path, llvm::make_unique<SymbolSlab>(std::move(Contents.first)),
llvm::make_unique<RefSlab>(std::move(Contents.second)));
- MainFileIndex.reset(MainFileSymbols.buildMemIndex());
+ MainFileIndex.reset(MainFileSymbols.buildIndex(IndexType::Light, URISchemes));
}
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/index/FileIndex.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.h?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/FileIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.h Tue Oct 16 01:53:52 2018
@@ -26,6 +26,14 @@
namespace clang {
namespace clangd {
+/// Select between in-memory index implementations, which have tradeoffs.
+enum class IndexType {
+ // MemIndex is trivially cheap to build, but has poor query performance.
+ Light,
+ // Dex is relatively expensive to build and uses more memory, but is fast.
+ Heavy,
+};
+
/// A container of Symbols from several source files. It can be updated
/// at source-file granularity, replacing all symbols from one file with a new
/// set.
@@ -47,7 +55,8 @@ public:
std::unique_ptr<RefSlab> Refs);
// The index keeps the symbols alive.
- std::unique_ptr<SymbolIndex> buildMemIndex();
+ std::unique_ptr<SymbolIndex>
+ buildIndex(IndexType, ArrayRef<std::string> URISchemes = {});
private:
mutable std::mutex Mutex;
@@ -64,7 +73,7 @@ class FileIndex : public MergedIndex {
public:
/// If URISchemes is empty, the default schemes in SymbolCollector will be
/// used.
- FileIndex(std::vector<std::string> URISchemes = {});
+ FileIndex(std::vector<std::string> URISchemes = {}, bool UseDex = true);
/// Update preamble symbols of file \p Path with all declarations in \p AST
/// and macros in \p PP.
@@ -76,6 +85,7 @@ public:
void updateMain(PathRef Path, ParsedAST &AST);
private:
+ bool UseDex; // FIXME: this should be always on.
std::vector<std::string> URISchemes;
// Contains information from each file's preamble only.
Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.h?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Dex.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.h Tue Oct 16 01:53:52 2018
@@ -52,8 +52,10 @@ public:
SymbolCollector::Options Opts;
URISchemes = Opts.URISchemes;
}
+ llvm::DenseSet<SymbolID> SeenIDs;
for (auto &&Sym : Symbols)
- this->Symbols.push_back(&Sym);
+ if (SeenIDs.insert(Sym.ID).second)
+ this->Symbols.push_back(&Sym);
for (auto &&Ref : Refs)
this->Refs.try_emplace(Ref.first, Ref.second);
buildIndex();
Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Oct 16 01:53:52 2018
@@ -29,11 +29,11 @@
using namespace clang;
using namespace clang::clangd;
-// FIXME: remove this option when Dex is stable enough.
+// FIXME: remove this option when Dex is cheap enough.
static llvm::cl::opt<bool>
UseDex("use-dex-index",
- llvm::cl::desc("Use experimental Dex static index."),
- llvm::cl::init(true), llvm::cl::Hidden);
+ llvm::cl::desc("Use experimental Dex dynamic index."),
+ llvm::cl::init(false), llvm::cl::Hidden);
static llvm::cl::opt<Path> CompileCommandsDir(
"compile-commands-dir",
@@ -286,6 +286,7 @@ int main(int argc, char *argv[]) {
if (!ResourceDir.empty())
Opts.ResourceDir = ResourceDir;
Opts.BuildDynamicSymbolIndex = EnableIndex;
+ Opts.HeavyweightDynamicSymbolIndex = UseDex;
std::unique_ptr<SymbolIndex> StaticIdx;
std::future<void> AsyncIndexLoad; // Block exit while loading the index.
if (EnableIndex && !IndexFile.empty()) {
@@ -293,7 +294,7 @@ int main(int argc, char *argv[]) {
SwapIndex *Placeholder;
StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique<MemIndex>()));
AsyncIndexLoad = runAsync<void>([Placeholder, &Opts] {
- if (auto Idx = loadIndex(IndexFile, Opts.URISchemes, UseDex))
+ if (auto Idx = loadIndex(IndexFile, Opts.URISchemes, /*UseDex=*/true))
Placeholder->reset(std::move(Idx));
});
if (RunSynchronously)
Modified: clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexTests.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexTests.cpp Tue Oct 16 01:53:52 2018
@@ -492,19 +492,13 @@ TEST(Dex, FuzzyFind) {
"other::A"));
}
-// FIXME(kbobyrev): This test is different for Dex and MemIndex: while
-// MemIndex manages response deduplication, Dex simply returns all matched
-// symbols which means there might be equivalent symbols in the response.
-// Before drop-in replacement of MemIndex with Dex happens, FileIndex
-// should handle deduplication instead.
TEST(DexTest, DexDeduplicate) {
std::vector<Symbol> Symbols = {symbol("1"), symbol("2"), symbol("3"),
symbol("2") /* duplicate */};
FuzzyFindRequest Req;
Req.Query = "2";
Dex I(Symbols, RefSlab(), URISchemes);
- EXPECT_FALSE(Req.Limit);
- EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
+ EXPECT_THAT(match(I, Req), ElementsAre("2"));
}
TEST(DexTest, DexLimitedNumMatches) {
Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Tue Oct 16 01:53:52 2018
@@ -82,12 +82,12 @@ RefSlab getRefs(const SymbolIndex &I, Sy
TEST(FileSymbolsTest, UpdateAndGet) {
FileSymbols FS;
- EXPECT_THAT(runFuzzyFind(*FS.buildMemIndex(), ""), IsEmpty());
+ EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""), IsEmpty());
FS.update("f1", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cc"));
- EXPECT_THAT(runFuzzyFind(*FS.buildMemIndex(), ""),
+ EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""),
UnorderedElementsAre(QName("1"), QName("2"), QName("3")));
- EXPECT_THAT(getRefs(*FS.buildMemIndex(), SymbolID("1")),
+ EXPECT_THAT(getRefs(*FS.buildIndex(IndexType::Light), SymbolID("1")),
RefsAre({FileURI("f1.cc")}));
}
@@ -95,9 +95,10 @@ TEST(FileSymbolsTest, Overlap) {
FileSymbols FS;
FS.update("f1", numSlab(1, 3), nullptr);
FS.update("f2", numSlab(3, 5), nullptr);
- EXPECT_THAT(runFuzzyFind(*FS.buildMemIndex(), ""),
- UnorderedElementsAre(QName("1"), QName("2"), QName("3"),
- QName("4"), QName("5")));
+ for (auto Type : {IndexType::Light, IndexType::Heavy})
+ EXPECT_THAT(runFuzzyFind(*FS.buildIndex(Type), ""),
+ UnorderedElementsAre(QName("1"), QName("2"), QName("3"),
+ QName("4"), QName("5")));
}
TEST(FileSymbolsTest, SnapshotAliveAfterRemove) {
@@ -106,13 +107,13 @@ TEST(FileSymbolsTest, SnapshotAliveAfter
SymbolID ID("1");
FS.update("f1", numSlab(1, 3), refSlab(ID, "f1.cc"));
- auto Symbols = FS.buildMemIndex();
+ auto Symbols = FS.buildIndex(IndexType::Light);
EXPECT_THAT(runFuzzyFind(*Symbols, ""),
UnorderedElementsAre(QName("1"), QName("2"), QName("3")));
EXPECT_THAT(getRefs(*Symbols, ID), RefsAre({FileURI("f1.cc")}));
FS.update("f1", nullptr, nullptr);
- auto Empty = FS.buildMemIndex();
+ auto Empty = FS.buildIndex(IndexType::Light);
EXPECT_THAT(runFuzzyFind(*Empty, ""), IsEmpty());
EXPECT_THAT(getRefs(*Empty, ID), ElementsAre());
Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestTU.cpp?rev=344594&r1=344593&r2=344594&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp Tue Oct 16 01:53:52 2018
@@ -50,7 +50,8 @@ SymbolSlab TestTU::headerSymbols() const
std::unique_ptr<SymbolIndex> TestTU::index() const {
auto AST = build();
- auto Idx = llvm::make_unique<FileIndex>();
+ auto Idx = llvm::make_unique<FileIndex>(
+ /*URISchemes=*/std::vector<std::string>{}, /*UseDex=*/true);
Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
Idx->updateMain(Filename, AST);
return std::move(Idx);
More information about the cfe-commits
mailing list