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