sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous,
MaskRay, ioeric, ilya-biryukov.
This is now handled by a wrapper class SwapIndex, so MemIndex/DexIndex can be
immutable and focus on their job.
Old and busted:
I have a MemIndex, which holds a shared_ptr<vector<Symbol*>>, which keeps the
symbol slab alive. I update by calling build(shared_ptr<vector<Symbol*>>).
New hotness: I have a SwapIndex, which holds a shared_ptr<SymbolIndex>, which
holds a MemIndex and also keeps any data backing it alive.
I update by building a new MemIndex and calling SwapIndex::reset().
This resulted in a bunch of interface churn (some places previously using
unique_ptr should now be shared_ptr, and some using MemIndex() + MemIndex::build
should now call the static MemIndex::build factory instead).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51422
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/MemIndex.h
clangd/index/dex/DexIndex.cpp
clangd/index/dex/DexIndex.h
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexIndexTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/TestIndex.cpp
unittests/clangd/TestIndex.h
unittests/clangd/TestTU.cpp
unittests/clangd/TestTU.h
Index: unittests/clangd/TestTU.h
===================================================================
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -51,7 +51,7 @@
ParsedAST build() const;
SymbolSlab headerSymbols() const;
- std::unique_ptr<SymbolIndex> index() const;
+ std::shared_ptr<SymbolIndex> index() const;
};
// Look up an index symbol by qualified name, which must be unique.
Index: unittests/clangd/TestTU.cpp
===================================================================
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -48,7 +48,7 @@
return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
}
-std::unique_ptr<SymbolIndex> TestTU::index() const {
+std::shared_ptr<SymbolIndex> TestTU::index() const {
return MemIndex::build(headerSymbols());
}
Index: unittests/clangd/TestIndex.h
===================================================================
--- unittests/clangd/TestIndex.h
+++ unittests/clangd/TestIndex.h
@@ -23,26 +23,11 @@
// Creates Symbol instance and sets SymbolID to given QualifiedName.
Symbol symbol(llvm::StringRef QName);
-// Bundles symbol pointers with the actual symbol slab the pointers refer to in
-// order to ensure that the slab isn't destroyed while it's used by and index.
-struct SlabAndPointers {
- SymbolSlab Slab;
- std::vector<const Symbol *> Pointers;
-};
+// Create a slab of symbols with the given qualified names as IDs and names.
+SymbolSlab generateSymbols(std::vector<std::string> QualifiedNames);
-// Create a slab of symbols with the given qualified names as both IDs and
-// names. The life time of the slab is managed by the returned shared pointer.
-// If \p WeakSymbols is provided, it will be pointed to the managed object in
-// the returned shared pointer.
-std::shared_ptr<std::vector<const Symbol *>>
-generateSymbols(std::vector<std::string> QualifiedNames,
- std::weak_ptr<SlabAndPointers> *WeakSymbols = nullptr);
-
-// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
-// to the `generateSymbols` above.
-std::shared_ptr<std::vector<const Symbol *>>
-generateNumSymbols(int Begin, int End,
- std::weak_ptr<SlabAndPointers> *WeakSymbols = nullptr);
+// Create a slab of symbols with IDs and names [Begin, End].
+SymbolSlab generateNumSymbols(int Begin, int End);
// Returns fully-qualified name out of given symbol.
std::string getQualifiedName(const Symbol &Sym);
Index: unittests/clangd/TestIndex.cpp
===================================================================
--- unittests/clangd/TestIndex.cpp
+++ unittests/clangd/TestIndex.cpp
@@ -26,30 +26,18 @@
return Sym;
}
-std::shared_ptr<std::vector<const Symbol *>>
-generateSymbols(std::vector<std::string> QualifiedNames,
- std::weak_ptr<SlabAndPointers> *WeakSymbols) {
+SymbolSlab generateSymbols(std::vector<std::string> QualifiedNames) {
SymbolSlab::Builder Slab;
for (llvm::StringRef QName : QualifiedNames)
Slab.insert(symbol(QName));
-
- auto Storage = std::make_shared<SlabAndPointers>();
- Storage->Slab = std::move(Slab).build();
- for (const auto &Sym : Storage->Slab)
- Storage->Pointers.push_back(&Sym);
- if (WeakSymbols)
- *WeakSymbols = Storage;
- auto *Pointers = &Storage->Pointers;
- return {std::move(Storage), Pointers};
+ return std::move(Slab).build();
}
-std::shared_ptr<std::vector<const Symbol *>>
-generateNumSymbols(int Begin, int End,
- std::weak_ptr<SlabAndPointers> *WeakSymbols) {
+SymbolSlab generateNumSymbols(int Begin, int End) {
std::vector<std::string> Names;
for (int i = Begin; i <= End; i++)
Names.push_back(std::to_string(i));
- return generateSymbols(Names, WeakSymbols);
+ return generateSymbols(Names);
}
std::string getQualifiedName(const Symbol &Sym) {
Index: unittests/clangd/IndexTests.cpp
===================================================================
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -14,8 +14,10 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+using testing::ElementsAre;
using testing::Pointee;
using testing::UnorderedElementsAre;
+using namespace llvm;
namespace clang {
namespace clangd {
@@ -39,152 +41,135 @@
EXPECT_THAT(*S.find(SymbolID(Sym)), Named(Sym));
}
-TEST(MemIndexTest, MemIndexSymbolsRecycled) {
- MemIndex I;
- std::weak_ptr<SlabAndPointers> Symbols;
- I.build(generateNumSymbols(0, 10, &Symbols));
- FuzzyFindRequest Req;
- Req.Query = "7";
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("7"));
+TEST(SwapIndexTest, OldIndexRecycled) {
+ auto A = std::make_shared<MemIndex>();
+ std::weak_ptr<SymbolIndex> WeakA = A;
- EXPECT_FALSE(Symbols.expired());
- // Release old symbols.
- I.build(generateNumSymbols(0, 0));
- EXPECT_TRUE(Symbols.expired());
+ SwapIndex S(std::move(A));
+ EXPECT_FALSE(WeakA.expired());
+ S.reset(std::make_shared<MemIndex>());
+ EXPECT_TRUE(WeakA.expired());
}
TEST(MemIndexTest, MemIndexDeduplicate) {
- auto Symbols = generateNumSymbols(0, 10);
-
- // Inject some duplicates and make sure we only match the same symbol once.
- auto Sym = symbol("7");
- Symbols->push_back(&Sym);
- Symbols->push_back(&Sym);
- Symbols->push_back(&Sym);
-
+ std::vector<Symbol> Symbols = {symbol("1"), symbol("2"), symbol("3"),
+ symbol("2") /* duplicate */};
FuzzyFindRequest Req;
- Req.Query = "7";
- MemIndex I;
- I.build(std::move(Symbols));
- auto Matches = match(I, Req);
- EXPECT_EQ(Matches.size(), 1u);
+ Req.Query = "2";
+ MemIndex I(Symbols);
+ EXPECT_THAT(match(I, Req), ElementsAre("2"));
}
TEST(MemIndexTest, MemIndexLimitedNumMatches) {
- MemIndex I;
- I.build(generateNumSymbols(0, 100));
+ auto I = MemIndex::build(generateNumSymbols(0, 100));
FuzzyFindRequest Req;
Req.Query = "5";
Req.MaxCandidateCount = 3;
bool Incomplete;
- auto Matches = match(I, Req, &Incomplete);
+ auto Matches = match(*I, Req, &Incomplete);
EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
EXPECT_TRUE(Incomplete);
}
TEST(MemIndexTest, FuzzyMatch) {
- MemIndex I;
- I.build(
+ auto I = MemIndex::build(
generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}));
FuzzyFindRequest Req;
Req.Query = "lol";
Req.MaxCandidateCount = 2;
- EXPECT_THAT(match(I, Req),
+ EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
TEST(MemIndexTest, MatchQualifiedNamesWithoutSpecificScope) {
- MemIndex I;
- I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
+ auto I = MemIndex::build(
+ generateSymbols({"a::y1", "b::y2", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
}
TEST(MemIndexTest, MatchQualifiedNamesWithGlobalScope) {
- MemIndex I;
- I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
+ auto I = MemIndex::build(
+ generateSymbols({"a::y1", "b::y2", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {""};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("y3"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("y3"));
}
TEST(MemIndexTest, MatchQualifiedNamesWithOneScope) {
- MemIndex I;
- I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}));
+ auto I = MemIndex::build(
+ generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2"));
}
TEST(MemIndexTest, MatchQualifiedNamesWithMultipleScopes) {
- MemIndex I;
- I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}));
+ auto I = MemIndex::build(
+ generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::", "b::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
}
TEST(MemIndexTest, NoMatchNestedScopes) {
- MemIndex I;
- I.build(generateSymbols({"a::y1", "a::b::y2"}));
+ auto I = MemIndex::build(
+ generateSymbols({"a::y1", "a::b::y2"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
TEST(MemIndexTest, IgnoreCases) {
- MemIndex I;
- I.build(generateSymbols({"ns::ABC", "ns::abc"}));
+ auto I = MemIndex::build(
+ generateSymbols({"ns::ABC", "ns::abc"}));
FuzzyFindRequest Req;
Req.Query = "AB";
Req.Scopes = {"ns::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
}
TEST(MemIndexTest, Lookup) {
- MemIndex I;
- I.build(generateSymbols({"ns::abc", "ns::xyz"}));
- EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
- EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
+ auto I = MemIndex::build(
+ generateSymbols({"ns::abc", "ns::xyz"}));
+ EXPECT_THAT(lookup(*I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
+ EXPECT_THAT(lookup(*I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::abc", "ns::xyz"));
- EXPECT_THAT(lookup(I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
+ EXPECT_THAT(lookup(*I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::xyz"));
- EXPECT_THAT(lookup(I, SymbolID("ns::nonono")), UnorderedElementsAre());
+ EXPECT_THAT(lookup(*I, SymbolID("ns::nonono")), UnorderedElementsAre());
}
TEST(MergeIndexTest, Lookup) {
- MemIndex I, J;
- I.build(generateSymbols({"ns::A", "ns::B"}));
- J.build(generateSymbols({"ns::B", "ns::C"}));
- EXPECT_THAT(lookup(*mergeIndex(&I, &J), SymbolID("ns::A")),
+ auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"})),
+ J = MemIndex::build(generateSymbols({"ns::B", "ns::C"}));
+ auto M = mergeIndex(I.get(), J.get());
+ EXPECT_THAT(lookup(*M, SymbolID("ns::A")),
UnorderedElementsAre("ns::A"));
- EXPECT_THAT(lookup(*mergeIndex(&I, &J), SymbolID("ns::B")),
+ EXPECT_THAT(lookup(*M, SymbolID("ns::B")),
UnorderedElementsAre("ns::B"));
- EXPECT_THAT(lookup(*mergeIndex(&I, &J), SymbolID("ns::C")),
+ EXPECT_THAT(lookup(*M, SymbolID("ns::C")),
UnorderedElementsAre("ns::C"));
- EXPECT_THAT(
- lookup(*mergeIndex(&I, &J), {SymbolID("ns::A"), SymbolID("ns::B")}),
- UnorderedElementsAre("ns::A", "ns::B"));
- EXPECT_THAT(
- lookup(*mergeIndex(&I, &J), {SymbolID("ns::A"), SymbolID("ns::C")}),
- UnorderedElementsAre("ns::A", "ns::C"));
- EXPECT_THAT(lookup(*mergeIndex(&I, &J), SymbolID("ns::D")),
+ EXPECT_THAT(lookup(*M, {SymbolID("ns::A"), SymbolID("ns::B")}),
+ UnorderedElementsAre("ns::A", "ns::B"));
+ EXPECT_THAT(lookup(*M, {SymbolID("ns::A"), SymbolID("ns::C")}),
+ UnorderedElementsAre("ns::A", "ns::C"));
+ EXPECT_THAT(lookup(*M, SymbolID("ns::D")),
UnorderedElementsAre());
- EXPECT_THAT(lookup(*mergeIndex(&I, &J), {}), UnorderedElementsAre());
+ EXPECT_THAT(lookup(*M, {}), UnorderedElementsAre());
}
TEST(MergeIndexTest, FuzzyFind) {
- MemIndex I, J;
- I.build(generateSymbols({"ns::A", "ns::B"}));
- J.build(generateSymbols({"ns::B", "ns::C"}));
+ auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"})),
+ J = MemIndex::build(generateSymbols({"ns::B", "ns::C"}));
FuzzyFindRequest Req;
Req.Scopes = {"ns::"};
- EXPECT_THAT(match(*mergeIndex(&I, &J), Req),
+ EXPECT_THAT(match(*mergeIndex(I.get(), J.get()), Req),
UnorderedElementsAre("ns::A", "ns::B", "ns::C"));
}
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -39,41 +39,44 @@
return llvm::make_unique<SymbolSlab>(std::move(Slab).build());
}
-std::vector<std::string>
-getSymbolNames(const std::vector<const Symbol *> &Symbols) {
+std::vector<std::string> getSymbolNames(const SymbolIndex &I,
+ std::string Query = "") {
+ FuzzyFindRequest Req;
+ Req.Query = Query;
std::vector<std::string> Names;
- for (const Symbol *Sym : Symbols)
- Names.push_back(Sym->Name);
+ I.fuzzyFind(Req, [&](const Symbol& S) {
+ Names.push_back(S.Name);
+ });
return Names;
}
TEST(FileSymbolsTest, UpdateAndGet) {
FileSymbols FS;
- EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre());
+ EXPECT_THAT(getSymbolNames(*FS.buildMemIndex()), UnorderedElementsAre());
FS.update("f1", numSlab(1, 3));
- EXPECT_THAT(getSymbolNames(*FS.allSymbols()),
+ EXPECT_THAT(getSymbolNames(*FS.buildMemIndex()),
UnorderedElementsAre("1", "2", "3"));
}
TEST(FileSymbolsTest, Overlap) {
FileSymbols FS;
FS.update("f1", numSlab(1, 3));
FS.update("f2", numSlab(3, 5));
- EXPECT_THAT(getSymbolNames(*FS.allSymbols()),
- UnorderedElementsAre("1", "2", "3", "3", "4", "5"));
+ EXPECT_THAT(getSymbolNames(*FS.buildMemIndex()),
+ UnorderedElementsAre("1", "2", "3", "4", "5"));
}
TEST(FileSymbolsTest, SnapshotAliveAfterRemove) {
FileSymbols FS;
FS.update("f1", numSlab(1, 3));
- auto Symbols = FS.allSymbols();
+ auto Symbols = FS.buildMemIndex();
EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3"));
FS.update("f1", nullptr);
- EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre());
+ EXPECT_THAT(getSymbolNames(*FS.buildMemIndex()), UnorderedElementsAre());
EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3"));
}
Index: unittests/clangd/DexIndexTests.cpp
===================================================================
--- unittests/clangd/DexIndexTests.cpp
+++ unittests/clangd/DexIndexTests.cpp
@@ -23,6 +23,7 @@
using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;
+using namespace llvm;
namespace clang {
namespace clangd {
@@ -408,171 +409,145 @@
}
TEST(DexIndex, Lookup) {
- DexIndex I;
- I.build(generateSymbols({"ns::abc", "ns::xyz"}));
- EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
- EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
+ auto I = DexIndex::build(generateSymbols({"ns::abc", "ns::xyz"}));
+ EXPECT_THAT(lookup(*I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
+ EXPECT_THAT(lookup(*I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::abc", "ns::xyz"));
- EXPECT_THAT(lookup(I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
+ EXPECT_THAT(lookup(*I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::xyz"));
- EXPECT_THAT(lookup(I, SymbolID("ns::nonono")), UnorderedElementsAre());
+ EXPECT_THAT(lookup(*I, SymbolID("ns::nonono")), UnorderedElementsAre());
}
TEST(DexIndex, FuzzyFind) {
- DexIndex Index;
- Index.build(generateSymbols({"ns::ABC", "ns::BCD", "::ABC", "ns::nested::ABC",
- "other::ABC", "other::A"}));
+ auto Index = DexIndex::build(
+ generateSymbols({"ns::ABC", "ns::BCD", "::ABC", "ns::nested::ABC",
+ "other::ABC", "other::A"}));
FuzzyFindRequest Req;
Req.Query = "ABC";
Req.Scopes = {"ns::"};
- EXPECT_THAT(match(Index, Req), UnorderedElementsAre("ns::ABC"));
+ EXPECT_THAT(match(*Index, Req), UnorderedElementsAre("ns::ABC"));
Req.Scopes = {"ns::", "ns::nested::"};
- EXPECT_THAT(match(Index, Req),
+ EXPECT_THAT(match(*Index, Req),
UnorderedElementsAre("ns::ABC", "ns::nested::ABC"));
Req.Query = "A";
Req.Scopes = {"other::"};
- EXPECT_THAT(match(Index, Req),
+ EXPECT_THAT(match(*Index, Req),
UnorderedElementsAre("other::A", "other::ABC"));
Req.Query = "";
Req.Scopes = {};
- EXPECT_THAT(match(Index, Req),
+ EXPECT_THAT(match(*Index, Req),
UnorderedElementsAre("ns::ABC", "ns::BCD", "::ABC",
"ns::nested::ABC", "other::ABC",
"other::A"));
}
TEST(DexIndexTest, FuzzyMatchQ) {
- DexIndex I;
- I.build(
+ auto I = DexIndex::build(
generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}));
FuzzyFindRequest Req;
Req.Query = "lol";
Req.MaxCandidateCount = 2;
- EXPECT_THAT(match(I, Req),
+ EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
-TEST(DexIndexTest, DexIndexSymbolsRecycled) {
- DexIndex I;
- std::weak_ptr<SlabAndPointers> Symbols;
- I.build(generateNumSymbols(0, 10, &Symbols));
- FuzzyFindRequest Req;
- Req.Query = "7";
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("7"));
-
- EXPECT_FALSE(Symbols.expired());
- // Release old symbols.
- I.build(generateNumSymbols(0, 0));
- EXPECT_TRUE(Symbols.expired());
-}
-
// FIXME(kbobyrev): This test is different for DexIndex and MemIndex: while
// MemIndex manages response deduplication, DexIndex simply returns all matched
// symbols which means there might be equivalent symbols in the response.
// Before drop-in replacement of MemIndex with DexIndex happens, FileIndex
// should handle deduplication instead.
TEST(DexIndexTest, DexIndexDeduplicate) {
- auto Symbols = generateNumSymbols(0, 10);
-
- // Inject duplicates.
- auto Sym = symbol("7");
- Symbols->push_back(&Sym);
- Symbols->push_back(&Sym);
- Symbols->push_back(&Sym);
-
+ std::vector<Symbol> Symbols = {symbol("1"), symbol("2"), symbol("3"),
+ symbol("2") /* duplicate */};
FuzzyFindRequest Req;
- Req.Query = "7";
- DexIndex I;
- I.build(std::move(Symbols));
- auto Matches = match(I, Req);
- EXPECT_EQ(Matches.size(), 4u);
+ Req.Query = "2";
+ DexIndex I(Symbols);
+ EXPECT_THAT(match(I, Req), ElementsAre("2","2"));
}
TEST(DexIndexTest, DexIndexLimitedNumMatches) {
- DexIndex I;
- I.build(generateNumSymbols(0, 100));
+ auto I = DexIndex::build(generateNumSymbols(0, 100));
FuzzyFindRequest Req;
Req.Query = "5";
Req.MaxCandidateCount = 3;
bool Incomplete;
- auto Matches = match(I, Req, &Incomplete);
+ auto Matches = match(*I, Req, &Incomplete);
EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
EXPECT_TRUE(Incomplete);
}
TEST(DexIndexTest, FuzzyMatch) {
- DexIndex I;
- I.build(
+ auto I = DexIndex::build(
generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}));
FuzzyFindRequest Req;
Req.Query = "lol";
Req.MaxCandidateCount = 2;
- EXPECT_THAT(match(I, Req),
+ EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
TEST(DexIndexTest, MatchQualifiedNamesWithoutSpecificScope) {
- DexIndex I;
- I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
+ auto I = DexIndex::build(
+ generateSymbols({"a::y1", "b::y2", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
}
TEST(DexIndexTest, MatchQualifiedNamesWithGlobalScope) {
- DexIndex I;
- I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
+ auto I = DexIndex::build(
+ generateSymbols({"a::y1", "b::y2", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {""};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("y3"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("y3"));
}
TEST(DexIndexTest, MatchQualifiedNamesWithOneScope) {
- DexIndex I;
- I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}));
+ auto I = DexIndex::build(
+ generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2"));
}
TEST(DexIndexTest, MatchQualifiedNamesWithMultipleScopes) {
- DexIndex I;
- I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}));
+ auto I = DexIndex::build(
+ generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::", "b::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
}
TEST(DexIndexTest, NoMatchNestedScopes) {
- DexIndex I;
- I.build(generateSymbols({"a::y1", "a::b::y2"}));
+ auto I = DexIndex::build(
+ generateSymbols({"a::y1", "a::b::y2"}));
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
TEST(DexIndexTest, IgnoreCases) {
- DexIndex I;
- I.build(generateSymbols({"ns::ABC", "ns::abc"}));
+ auto I = DexIndex::build(
+ generateSymbols({"ns::ABC", "ns::abc"}));
FuzzyFindRequest Req;
Req.Query = "AB";
Req.Scopes = {"ns::"};
- EXPECT_THAT(match(I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
+ EXPECT_THAT(match(*I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
}
TEST(DexIndexTest, Lookup) {
- DexIndex I;
- I.build(generateSymbols({"ns::abc", "ns::xyz"}));
- EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
- EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
+ auto I = DexIndex::build(
+ generateSymbols({"ns::abc", "ns::xyz"}));
+ EXPECT_THAT(lookup(*I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
+ EXPECT_THAT(lookup(*I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::abc", "ns::xyz"));
- EXPECT_THAT(lookup(I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
+ EXPECT_THAT(lookup(*I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::xyz"));
- EXPECT_THAT(lookup(I, SymbolID("ns::nonono")), UnorderedElementsAre());
+ EXPECT_THAT(lookup(*I, SymbolID("ns::nonono")), UnorderedElementsAre());
}
} // namespace
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -72,7 +72,7 @@
}
MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
-std::unique_ptr<SymbolIndex> memIndex(std::vector<Symbol> Symbols) {
+std::shared_ptr<SymbolIndex> memIndex(std::vector<Symbol> Symbols) {
SymbolSlab::Builder Slab;
for (const auto &Sym : Symbols)
Slab.insert(Sym);
@@ -83,7 +83,7 @@
Position point,
std::vector<Symbol> IndexSymbols = {},
clangd::CodeCompleteOptions Opts = {}) {
- std::unique_ptr<SymbolIndex> OverrideIndex;
+ std::shared_ptr<SymbolIndex> OverrideIndex;
if (!IndexSymbols.empty()) {
assert(!Opts.Index && "both Index and IndexSymbols given!");
OverrideIndex = memIndex(std::move(IndexSymbols));
@@ -99,7 +99,7 @@
CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
std::vector<Symbol> IndexSymbols = {},
clangd::CodeCompleteOptions Opts = {}) {
- std::unique_ptr<SymbolIndex> OverrideIndex;
+ std::shared_ptr<SymbolIndex> OverrideIndex;
if (!IndexSymbols.empty()) {
assert(!Opts.Index && "both Index and IndexSymbols given!");
OverrideIndex = memIndex(std::move(IndexSymbols));
@@ -828,7 +828,7 @@
SignatureHelp signatures(StringRef Text,
std::vector<Symbol> IndexSymbols = {}) {
- std::unique_ptr<SymbolIndex> Index;
+ std::shared_ptr<SymbolIndex> Index;
if (!IndexSymbols.empty())
Index = memIndex(IndexSymbols);
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -42,7 +42,7 @@
// Build an in-memory static index for global symbols from a YAML-format file.
// The size of global symbols should be relatively small, so that all symbols
// can be managed in memory.
-std::unique_ptr<SymbolIndex> buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+std::shared_ptr<SymbolIndex> buildStaticIndex(llvm::StringRef YamlSymbolFile) {
auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
if (!Buffer) {
llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
@@ -296,7 +296,7 @@
if (!ResourceDir.empty())
Opts.ResourceDir = ResourceDir;
Opts.BuildDynamicSymbolIndex = EnableIndex;
- std::unique_ptr<SymbolIndex> StaticIdx;
+ std::shared_ptr<SymbolIndex> StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
StaticIdx = buildStaticIndex(YamlSymbolFile);
Opts.StaticIndex = StaticIdx.get();
Index: clangd/index/dex/DexIndex.h
===================================================================
--- clangd/index/dex/DexIndex.h
+++ clangd/index/dex/DexIndex.h
@@ -25,7 +25,6 @@
#include "Iterator.h"
#include "Token.h"
#include "Trigram.h"
-#include <mutex>
namespace clang {
namespace clangd {
@@ -39,12 +38,15 @@
// index on disk and then load it if available.
class DexIndex : public SymbolIndex {
public:
- /// \brief (Re-)Build index for `Symbols`. All symbol pointers must remain
- /// accessible as long as `Symbols` is kept alive.
- void build(std::shared_ptr<std::vector<const Symbol *>> Symbols);
+ // All symbols must outlive this index.
+ template <typename Range> DexIndex(Range &&Symbols) {
+ for (auto &&Sym : Symbols)
+ this->Symbols.push_back(&Sym);
+ buildIndex();
+ }
- /// \brief Build index from a symbol slab.
- static std::unique_ptr<SymbolIndex> build(SymbolSlab Slab);
+ /// Builds an index from a slab. The shared_ptr manages the slab's lifetime.
+ static std::shared_ptr<SymbolIndex> build(SymbolSlab Slab);
bool
fuzzyFind(const FuzzyFindRequest &Req,
@@ -60,19 +62,18 @@
size_t estimateMemoryUsage() const override;
private:
+ void buildIndex();
- mutable std::mutex Mutex;
-
- std::shared_ptr<std::vector<const Symbol *>> Symbols /*GUARDED_BY(Mutex)*/;
- llvm::DenseMap<SymbolID, const Symbol *> LookupTable /*GUARDED_BY(Mutex)*/;
- llvm::DenseMap<const Symbol *, float> SymbolQuality /*GUARDED_BY(Mutex)*/;
+ std::vector<const Symbol *> Symbols;
+ llvm::DenseMap<SymbolID, const Symbol *> LookupTable;
+ llvm::DenseMap<const Symbol *, float> SymbolQuality;
// Inverted index is a mapping from the search token to the posting list,
// which contains all items which can be characterized by such search token.
// For example, if the search token is scope "std::", the corresponding
// posting list would contain all indices of symbols defined in namespace std.
// Inverted index is used to retrieve posting lists which are processed during
// the fuzzyFind process.
- llvm::DenseMap<Token, PostingList> InvertedIndex /*GUARDED_BY(Mutex)*/;
+ llvm::DenseMap<Token, PostingList> InvertedIndex;
};
} // namespace dex
Index: clangd/index/dex/DexIndex.cpp
===================================================================
--- clangd/index/dex/DexIndex.cpp
+++ clangd/index/dex/DexIndex.cpp
@@ -36,46 +36,40 @@
} // namespace
-void DexIndex::build(std::shared_ptr<std::vector<const Symbol *>> Syms) {
- llvm::DenseMap<SymbolID, const Symbol *> TempLookupTable;
- llvm::DenseMap<const Symbol *, float> TempSymbolQuality;
- for (const Symbol *Sym : *Syms) {
- TempLookupTable[Sym->ID] = Sym;
- TempSymbolQuality[Sym] = quality(*Sym);
+void DexIndex::buildIndex() {
+ for (const Symbol *Sym : Symbols) {
+ LookupTable[Sym->ID] = Sym;
+ SymbolQuality[Sym] = quality(*Sym);
}
// Symbols are sorted by symbol qualities so that items in the posting lists
// are stored in the descending order of symbol quality.
- std::sort(begin(*Syms), end(*Syms),
+ std::sort(begin(Symbols), end(Symbols),
[&](const Symbol *LHS, const Symbol *RHS) {
- return TempSymbolQuality[LHS] > TempSymbolQuality[RHS];
+ return SymbolQuality[LHS] > SymbolQuality[RHS];
});
- llvm::DenseMap<Token, PostingList> TempInvertedIndex;
+
// Populate TempInvertedIndex with posting lists for index symbols.
- for (DocID SymbolRank = 0; SymbolRank < Syms->size(); ++SymbolRank) {
- const auto *Sym = (*Syms)[SymbolRank];
+ for (DocID SymbolRank = 0; SymbolRank < Symbols.size(); ++SymbolRank) {
+ const auto *Sym = Symbols[SymbolRank];
for (const auto &Token : generateSearchTokens(*Sym))
- TempInvertedIndex[Token].push_back(SymbolRank);
- }
-
- {
- std::lock_guard<std::mutex> Lock(Mutex);
-
- // Replace outdated index with the new one.
- LookupTable = std::move(TempLookupTable);
- Symbols = std::move(Syms);
- InvertedIndex = std::move(TempInvertedIndex);
- SymbolQuality = std::move(TempSymbolQuality);
+ InvertedIndex[Token].push_back(SymbolRank);
}
vlog("Built DexIndex with estimated memory usage {0} bytes.",
estimateMemoryUsage());
}
-std::unique_ptr<SymbolIndex> DexIndex::build(SymbolSlab Slab) {
- auto Idx = llvm::make_unique<DexIndex>();
- Idx->build(getSymbolsFromSlab(std::move(Slab)));
- return std::move(Idx);
+std::shared_ptr<SymbolIndex> DexIndex::build(SymbolSlab Slab) {
+ struct IndexAndSlab {
+ IndexAndSlab(SymbolSlab Slab)
+ : Index(llvm::make_range(Slab.begin(), Slab.end())),
+ Slab(std::move(Slab)) {}
+ DexIndex Index;
+ SymbolSlab Slab;
+ };
+ auto Storage = std::make_shared<IndexAndSlab>(std::move(Slab));
+ return std::shared_ptr<SymbolIndex>(std::move(Storage), &Storage->Index);
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -92,75 +86,69 @@
std::vector<std::unique_ptr<Iterator>> TopLevelChildren;
const auto TrigramTokens = generateIdentifierTrigrams(Req.Query);
- {
- std::lock_guard<std::mutex> Lock(Mutex);
-
- // Generate query trigrams and construct AND iterator over all query
- // trigrams.
- std::vector<std::unique_ptr<Iterator>> TrigramIterators;
- for (const auto &Trigram : TrigramTokens) {
- const auto It = InvertedIndex.find(Trigram);
- if (It != InvertedIndex.end())
- TrigramIterators.push_back(create(It->second));
- }
- if (!TrigramIterators.empty())
- TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
-
- // Generate scope tokens for search query.
- std::vector<std::unique_ptr<Iterator>> ScopeIterators;
- for (const auto &Scope : Req.Scopes) {
- const auto It = InvertedIndex.find(Token(Token::Kind::Scope, Scope));
- if (It != InvertedIndex.end())
- ScopeIterators.push_back(create(It->second));
- }
- // Add OR iterator for scopes if there are any Scope Iterators.
- if (!ScopeIterators.empty())
- TopLevelChildren.push_back(createOr(move(ScopeIterators)));
-
- // Use TRUE iterator if both trigrams and scopes from the query are not
- // present in the symbol index.
- auto QueryIterator = TopLevelChildren.empty()
- ? createTrue(Symbols->size())
- : createAnd(move(TopLevelChildren));
- // Retrieve more items than it was requested: some of the items with high
- // final score might not be retrieved otherwise.
- // FIXME(kbobyrev): Pre-scoring retrieval threshold should be adjusted as
- // using 100x of the requested number might not be good in practice, e.g.
- // when the requested number of items is small.
- const unsigned ItemsToRetrieve = 100 * Req.MaxCandidateCount;
- auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);
- // FIXME(kbobyrev): Add boosting to the query and utilize retrieved
- // boosting scores.
- std::vector<std::pair<DocID, float>> SymbolDocIDs = consume(*Root);
-
- // Retrieve top Req.MaxCandidateCount items.
- std::priority_queue<std::pair<float, const Symbol *>> Top;
- for (const auto &P : SymbolDocIDs) {
- const DocID SymbolDocID = P.first;
- const auto *Sym = (*Symbols)[SymbolDocID];
- const llvm::Optional<float> Score = Filter.match(Sym->Name);
- if (!Score)
- continue;
- // Multiply score by a negative factor so that Top stores items with the
- // highest actual score.
- Top.emplace(-(*Score) * SymbolQuality.find(Sym)->second, Sym);
- if (Top.size() > Req.MaxCandidateCount) {
- More = true;
- Top.pop();
- }
+ // Generate query trigrams and construct AND iterator over all query
+ // trigrams.
+ std::vector<std::unique_ptr<Iterator>> TrigramIterators;
+ for (const auto &Trigram : TrigramTokens) {
+ const auto It = InvertedIndex.find(Trigram);
+ if (It != InvertedIndex.end())
+ TrigramIterators.push_back(create(It->second));
+ }
+ if (!TrigramIterators.empty())
+ TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+
+ // Generate scope tokens for search query.
+ std::vector<std::unique_ptr<Iterator>> ScopeIterators;
+ for (const auto &Scope : Req.Scopes) {
+ const auto It = InvertedIndex.find(Token(Token::Kind::Scope, Scope));
+ if (It != InvertedIndex.end())
+ ScopeIterators.push_back(create(It->second));
+ }
+ // Add OR iterator for scopes if there are any Scope Iterators.
+ if (!ScopeIterators.empty())
+ TopLevelChildren.push_back(createOr(move(ScopeIterators)));
+
+ // Use TRUE iterator if both trigrams and scopes from the query are not
+ // present in the symbol index.
+ auto QueryIterator = TopLevelChildren.empty()
+ ? createTrue(Symbols.size())
+ : createAnd(move(TopLevelChildren));
+ // Retrieve more items than it was requested: some of the items with high
+ // final score might not be retrieved otherwise.
+ // FIXME(kbobyrev): Pre-scoring retrieval threshold should be adjusted as
+ // using 100x of the requested number might not be good in practice, e.g.
+ // when the requested number of items is small.
+ const unsigned ItemsToRetrieve = 100 * Req.MaxCandidateCount;
+ auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);
+ // FIXME(kbobyrev): Add boosting to the query and utilize retrieved
+ // boosting scores.
+ std::vector<std::pair<DocID, float>> SymbolDocIDs = consume(*Root);
+
+ // Retrieve top Req.MaxCandidateCount items.
+ std::priority_queue<std::pair<float, const Symbol *>> Top;
+ for (const auto &P : SymbolDocIDs) {
+ const DocID SymbolDocID = P.first;
+ const auto *Sym = Symbols[SymbolDocID];
+ const llvm::Optional<float> Score = Filter.match(Sym->Name);
+ if (!Score)
+ continue;
+ // Multiply score by a negative factor so that Top stores items with the
+ // highest actual score.
+ Top.emplace(-(*Score) * SymbolQuality.find(Sym)->second, Sym);
+ if (Top.size() > Req.MaxCandidateCount) {
+ More = true;
+ Top.pop();
}
-
- // Apply callback to the top Req.MaxCandidateCount items.
- for (; !Top.empty(); Top.pop())
- Callback(*Top.top().second);
}
+ // Apply callback to the top Req.MaxCandidateCount items.
+ for (; !Top.empty(); Top.pop())
+ Callback(*Top.top().second);
return More;
}
void DexIndex::lookup(const LookupRequest &Req,
llvm::function_ref<void(const Symbol &)> Callback) const {
- std::lock_guard<std::mutex> Lock(Mutex);
for (const auto &ID : Req.IDs) {
auto I = LookupTable.find(ID);
if (I != LookupTable.end())
@@ -175,8 +163,6 @@
}
size_t DexIndex::estimateMemoryUsage() const {
- std::lock_guard<std::mutex> Lock(Mutex);
-
size_t Bytes =
LookupTable.size() * sizeof(std::pair<SymbolID, const Symbol *>);
Bytes += SymbolQuality.size() * sizeof(std::pair<const Symbol *, float>);
Index: clangd/index/MemIndex.h
===================================================================
--- clangd/index/MemIndex.h
+++ clangd/index/MemIndex.h
@@ -16,16 +16,18 @@
namespace clang {
namespace clangd {
-/// \brief This implements an index for a (relatively small) set of symbols that
-/// can be easily managed in memory.
+/// MemIndex is a naive in-memory index suitable for a small set of symbols.
class MemIndex : public SymbolIndex {
public:
- /// \brief (Re-)Build index for `Symbols`. All symbol pointers must remain
- /// accessible as long as `Symbols` is kept alive.
- void build(std::shared_ptr<std::vector<const Symbol *>> Symbols);
+ // All symbols must outlive this index.
+ template <typename Range> MemIndex(Range &&Symbols) {
+ for (const Symbol &S : Symbols)
+ Index[S.ID] = &S;
+ }
+ MemIndex() = default;
- /// \brief Build index from a symbol slab.
- static std::unique_ptr<SymbolIndex> build(SymbolSlab Slab);
+ /// Builds an index from a slab. The shared_ptr manages the slab's lifetime.
+ static std::shared_ptr<SymbolIndex> build(SymbolSlab Slab);
bool
fuzzyFind(const FuzzyFindRequest &Req,
@@ -42,19 +44,10 @@
size_t estimateMemoryUsage() const override;
private:
-
- std::shared_ptr<std::vector<const Symbol *>> Symbols;
// Index is a set of symbols that are deduplicated by symbol IDs.
- // FIXME: build smarter index structure.
llvm::DenseMap<SymbolID, const Symbol *> Index;
- mutable std::mutex Mutex;
};
-// Returns pointers to the symbols in given slab and bundles slab lifetime with
-// returned symbol pointers so that the pointers are never invalid.
-std::shared_ptr<std::vector<const Symbol *>>
-getSymbolsFromSlab(SymbolSlab Slab);
-
} // namespace clangd
} // namespace clang
Index: clangd/index/MemIndex.cpp
===================================================================
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -15,26 +15,16 @@
namespace clang {
namespace clangd {
-void MemIndex::build(std::shared_ptr<std::vector<const Symbol *>> Syms) {
- llvm::DenseMap<SymbolID, const Symbol *> TempIndex;
- for (const Symbol *Sym : *Syms)
- TempIndex[Sym->ID] = Sym;
-
- // Swap out the old symbols and index.
- {
- std::lock_guard<std::mutex> Lock(Mutex);
- Index = std::move(TempIndex);
- Symbols = std::move(Syms); // Relase old symbols.
- }
-
- vlog("Built MemIndex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
-}
-
-std::unique_ptr<SymbolIndex> MemIndex::build(SymbolSlab Slab) {
- auto Idx = llvm::make_unique<MemIndex>();
- Idx->build(getSymbolsFromSlab(std::move(Slab)));
- return std::move(Idx);
+std::shared_ptr<SymbolIndex> MemIndex::build(SymbolSlab Slab) {
+ struct SlabAndIndex {
+ SlabAndIndex(SymbolSlab Slab)
+ : Index(llvm::make_range(Slab.begin(), Slab.end())),
+ Slab(std::move(Slab)) {}
+ MemIndex Index;
+ SymbolSlab Slab;
+ };
+ auto Payload = std::make_shared<SlabAndIndex>(std::move(Slab));
+ return std::shared_ptr<SymbolIndex>(std::move(Payload), &Payload->Index);
}
bool MemIndex::fuzzyFind(
@@ -46,34 +36,30 @@
std::priority_queue<std::pair<float, const Symbol *>> Top;
FuzzyMatcher Filter(Req.Query);
bool More = false;
- {
- std::lock_guard<std::mutex> Lock(Mutex);
- for (const auto Pair : Index) {
- const Symbol *Sym = Pair.second;
+ for (const auto Pair : Index) {
+ const Symbol *Sym = Pair.second;
- // Exact match against all possible scopes.
- if (!Req.Scopes.empty() && !llvm::is_contained(Req.Scopes, Sym->Scope))
- continue;
- if (Req.RestrictForCodeCompletion && !Sym->IsIndexedForCodeCompletion)
- continue;
+ // Exact match against all possible scopes.
+ if (!Req.Scopes.empty() && !llvm::is_contained(Req.Scopes, Sym->Scope))
+ continue;
+ if (Req.RestrictForCodeCompletion && !Sym->IsIndexedForCodeCompletion)
+ continue;
- if (auto Score = Filter.match(Sym->Name)) {
- Top.emplace(-*Score * quality(*Sym), Sym);
- if (Top.size() > Req.MaxCandidateCount) {
- More = true;
- Top.pop();
- }
+ if (auto Score = Filter.match(Sym->Name)) {
+ Top.emplace(-*Score * quality(*Sym), Sym);
+ if (Top.size() > Req.MaxCandidateCount) {
+ More = true;
+ Top.pop();
}
}
- for (; !Top.empty(); Top.pop())
- Callback(*Top.top().second);
}
+ for (; !Top.empty(); Top.pop())
+ Callback(*Top.top().second);
return More;
}
void MemIndex::lookup(const LookupRequest &Req,
llvm::function_ref<void(const Symbol &)> Callback) const {
- std::lock_guard<std::mutex> Lock(Mutex);
for (const auto &ID : Req.IDs) {
auto I = Index.find(ID);
if (I != Index.end())
@@ -87,22 +73,7 @@
log("findOccurrences is not implemented.");
}
-std::shared_ptr<std::vector<const Symbol *>>
-getSymbolsFromSlab(SymbolSlab Slab) {
- struct Snapshot {
- SymbolSlab Slab;
- std::vector<const Symbol *> Pointers;
- };
- auto Snap = std::make_shared<Snapshot>();
- Snap->Slab = std::move(Slab);
- for (auto &Sym : Snap->Slab)
- Snap->Pointers.push_back(&Sym);
- return std::shared_ptr<std::vector<const Symbol *>>(std::move(Snap),
- &Snap->Pointers);
-}
-
size_t MemIndex::estimateMemoryUsage() const {
- std::lock_guard<std::mutex> Lock(Mutex);
return Index.getMemorySize();
}
Index: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -19,6 +19,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/StringSaver.h"
#include <array>
+#include <mutex>
#include <string>
#include <tuple>
@@ -363,7 +364,7 @@
SymbolOccurrenceKind Filter;
};
-/// \brief Interface for symbol indexes that can be used for searching or
+/// Interface for symbol indexes that can be used for searching or
/// matching symbols among a set of symbols based on names or unique IDs.
class SymbolIndex {
public:
@@ -402,6 +403,31 @@
virtual size_t estimateMemoryUsage() const = 0;
};
+// Delegating implementation of SymbolIndex whose delegate can be swapped out.
+class SwapIndex : public SymbolIndex {
+public:
+ // If an index is not provided, reset() must be called.
+ SwapIndex(std::shared_ptr<SymbolIndex> Index = nullptr)
+ : Index(std::move(Index)) {}
+ void reset(std::shared_ptr<SymbolIndex>);
+
+ // SymbolIndex methods delegate to the current index, which is kept alive
+ // until the call returns (even if reset() is called).
+ bool fuzzyFind(const FuzzyFindRequest &,
+ llvm::function_ref<void(const Symbol &)>) const override;
+ void lookup(const LookupRequest &,
+ llvm::function_ref<void(const Symbol &)>) const override;
+ void findOccurrences(
+ const OccurrencesRequest &,
+ llvm::function_ref<void(const SymbolOccurrence &)>) const override;
+ size_t estimateMemoryUsage() const override;
+
+private:
+ std::shared_ptr<SymbolIndex> snapshot() const;
+ mutable std::mutex Mutex;
+ std::shared_ptr<SymbolIndex> Index;
+};
+
} // namespace clangd
} // namespace clang
Index: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -128,5 +128,36 @@
return SymbolSlab(std::move(NewArena), std::move(Symbols));
}
+void SwapIndex::reset(std::shared_ptr<SymbolIndex> Index) {
+ // Keep the old index alive, so we don't destroy it under lock (may be slow).
+ std::shared_ptr<SymbolIndex> Pin;
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ Pin = std::move(this->Index);
+ this->Index = std::move(Index);
+ }
+}
+std::shared_ptr<SymbolIndex> SwapIndex::snapshot() const {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ return Index;
+}
+
+bool SwapIndex::fuzzyFind(const FuzzyFindRequest &R,
+ llvm::function_ref<void(const Symbol &)> CB) const {
+ return snapshot()->fuzzyFind(R, CB);
+}
+void SwapIndex::lookup(const LookupRequest &R,
+ llvm::function_ref<void(const Symbol &)> CB) const {
+ return snapshot()->lookup(R, CB);
+}
+void SwapIndex::findOccurrences(
+ const OccurrencesRequest &R,
+ llvm::function_ref<void(const SymbolOccurrence &)> CB) const {
+ return snapshot()->findOccurrences(R, CB);
+}
+size_t SwapIndex::estimateMemoryUsage() const {
+ return snapshot()->estimateMemoryUsage();
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/index/FileIndex.h
===================================================================
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -24,7 +24,7 @@
namespace clang {
namespace clangd {
-/// \brief A container of Symbols from several source files. It can be updated
+/// 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.
///
@@ -39,28 +39,28 @@
/// locking when we swap or obtain refereces to snapshots.
class FileSymbols {
public:
- /// \brief Updates all symbols in a file. If \p Slab is nullptr, symbols for
+ /// Updates all symbols in a file. If \p Slab is nullptr, symbols for
/// \p Path will be removed.
void update(PathRef Path, std::unique_ptr<SymbolSlab> Slab);
- // The shared_ptr keeps the symbols alive
- std::shared_ptr<std::vector<const Symbol *>> allSymbols();
+ // The shared_ptr keeps the symbols alive.
+ std::shared_ptr<SymbolIndex> buildMemIndex();
private:
mutable std::mutex Mutex;
- /// \brief Stores the latest snapshots for all active files.
+ /// Stores the latest snapshots for all active files.
llvm::StringMap<std::shared_ptr<SymbolSlab>> FileToSlabs;
};
-/// \brief This manages symbls from files and an in-memory index on all symbols.
-class FileIndex : public SymbolIndex {
+/// This manages symbols from files and an in-memory index on all symbols.
+class FileIndex : public SwapIndex {
public:
/// If URISchemes is empty, the default schemes in SymbolCollector will be
/// used.
FileIndex(std::vector<std::string> URISchemes = {});
- /// \brief Update symbols in \p Path with symbols in \p AST. If \p AST is
+ /// Update symbols in \p Path with symbols in \p AST. If \p AST is
/// nullptr, this removes all symbols in the file.
/// If \p AST is not null, \p PP cannot be null and it should be the
/// preprocessor that was used to build \p AST.
@@ -70,23 +70,11 @@
update(PathRef Path, ASTContext *AST, std::shared_ptr<Preprocessor> PP,
llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls = llvm::None);
- bool
- fuzzyFind(const FuzzyFindRequest &Req,
- llvm::function_ref<void(const Symbol &)> Callback) const override;
-
- void lookup(const LookupRequest &Req,
- llvm::function_ref<void(const Symbol &)> Callback) const override;
-
-
- void findOccurrences(const OccurrencesRequest &Req,
- llvm::function_ref<void(const SymbolOccurrence &)>
- Callback) const override;
-
- size_t estimateMemoryUsage() const override;
-
private:
+ // Only update() should swap the index.
+ using SwapIndex::reset;
+
FileSymbols FSymbols;
- MemIndex Index;
std::vector<std::string> URISchemes;
};
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -52,7 +52,9 @@
}
FileIndex::FileIndex(std::vector<std::string> URISchemes)
- : URISchemes(std::move(URISchemes)) {}
+ : URISchemes(std::move(URISchemes)) {
+ reset(FSymbols.buildMemIndex());
+}
void FileSymbols::update(PathRef Path, std::unique_ptr<SymbolSlab> Slab) {
std::lock_guard<std::mutex> Lock(Mutex);
@@ -62,27 +64,27 @@
FileToSlabs[Path] = std::move(Slab);
}
-std::shared_ptr<std::vector<const Symbol *>> FileSymbols::allSymbols() {
+std::shared_ptr<SymbolIndex> FileSymbols::buildMemIndex() {
// The snapshot manages life time of symbol slabs and provides pointers of all
// symbols in all slabs.
struct Snapshot {
- std::vector<const Symbol *> Pointers;
- std::vector<std::shared_ptr<SymbolSlab>> KeepAlive;
+ llvm::Optional<MemIndex> Index;
+ std::vector<std::shared_ptr<SymbolSlab>> Slabs;
};
auto Snap = std::make_shared<Snapshot>();
{
std::lock_guard<std::mutex> Lock(Mutex);
-
- for (const auto &FileAndSlab : FileToSlabs) {
- Snap->KeepAlive.push_back(FileAndSlab.second);
- for (const auto &Iter : *FileAndSlab.second)
- Snap->Pointers.push_back(&Iter);
- }
+ for (const auto &FileAndSlab : FileToSlabs)
+ Snap->Slabs.push_back(FileAndSlab.second);
}
- auto *Pointers = &Snap->Pointers;
- // Use aliasing constructor to keep the snapshot alive along with the
- // pointers.
- return {std::move(Snap), Pointers};
+ std::vector<const Symbol*> AllSymbols;
+ for (const auto &Slab : Snap->Slabs)
+ for (const auto &Sym : *Slab)
+ AllSymbols.push_back(&Sym);
+ Snap->Index.emplace(llvm::make_pointee_range(AllSymbols));
+ // Use aliasing constructor to keep the slabs alive along with the index.
+ return std::shared_ptr<SymbolIndex>(std::move(Snap),
+ Snap->Index.getPointer());
}
void FileIndex::update(PathRef Path, ASTContext *AST,
@@ -96,30 +98,7 @@
*Slab = indexAST(*AST, PP, TopLevelDecls, URISchemes);
FSymbols.update(Path, std::move(Slab));
}
- auto Symbols = FSymbols.allSymbols();
- Index.build(std::move(Symbols));
-}
-
-bool FileIndex::fuzzyFind(
- const FuzzyFindRequest &Req,
- llvm::function_ref<void(const Symbol &)> Callback) const {
- return Index.fuzzyFind(Req, Callback);
-}
-
-void FileIndex::lookup(
- const LookupRequest &Req,
- llvm::function_ref<void(const Symbol &)> Callback) const {
- Index.lookup(Req, Callback);
-}
-
-void FileIndex::findOccurrences(
- const OccurrencesRequest &Req,
- llvm::function_ref<void(const SymbolOccurrence &)> Callback) const {
- log("findOccurrences is not implemented.");
-}
-
-size_t FileIndex::estimateMemoryUsage() const {
- return Index.estimateMemoryUsage();
+ reset(FSymbols.buildMemIndex());
}
} // namespace clangd
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
