diff options
author | Sam McCall <sam.mccall@gmail.com> | 2017-12-23 19:38:03 +0000 |
---|---|---|
committer | Sam McCall <sam.mccall@gmail.com> | 2017-12-23 19:38:03 +0000 |
commit | 057470116210d2e6bf5e726361e641c8c5de9b35 (patch) | |
tree | dcbb6eab3c191376a2f05c7a21e8a0d1a0137cc7 /unittests | |
parent | 13c71d1749f61820db211dace1146d9c5aa83c55 (diff) |
[clangd] Use Builder for symbol slabs, and use sorted-vector for storage
Summary:
This improves a few things:
- the insert -> freeze -> read sequence is now enforced/communicated by the
type system
- SymbolSlab::const_iterator iterates over symbols, not over id-symbol pairs
- we avoid permanently storing a second copy of the IDs, and the
string map's hashtable
The slab size is now down to 21.8MB for the LLVM project.
Of this only 2.7MB is strings, the rest is #symbols * `sizeof(Symbol)`.
`sizeof(Symbol)` is currently 96, which seems too big - I think
SymbolInfo isn't efficiently packed. That's a topic for another patch!
Also added simple API to see the memory usage/#symbols of a slab, since
it seems likely we will continue to care about this.
Reviewers: ilya-biryukov
Subscribers: klimek, mgrang, cfe-commits
Differential Revision: https://reviews.llvm.org/D41506
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@321412 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'unittests')
-rw-r--r-- | unittests/clangd/CodeCompleteTests.cpp | 6 | ||||
-rw-r--r-- | unittests/clangd/FileIndexTests.cpp | 31 | ||||
-rw-r--r-- | unittests/clangd/IndexTests.cpp | 40 | ||||
-rw-r--r-- | unittests/clangd/SymbolCollectorTests.cpp | 3 |
4 files changed, 42 insertions, 38 deletions
diff --git a/unittests/clangd/CodeCompleteTests.cpp b/unittests/clangd/CodeCompleteTests.cpp index db998b08..e3042c3d 100644 --- a/unittests/clangd/CodeCompleteTests.cpp +++ b/unittests/clangd/CodeCompleteTests.cpp @@ -449,6 +449,7 @@ std::unique_ptr<SymbolIndex> simpleIndexFromSymbols( std::vector<const Symbol *> Pointers; }; auto Snap = std::make_shared<Snapshot>(); + SymbolSlab::Builder Slab; for (const auto &Pair : Symbols) { Symbol Sym; Sym.ID = SymbolID(Pair.first); @@ -462,10 +463,11 @@ std::unique_ptr<SymbolIndex> simpleIndexFromSymbols( Sym.Scope = QName.substr(0, Pos); } Sym.SymInfo.Kind = Pair.second; - Snap->Slab.insert(std::move(Sym)); + Slab.insert(Sym); } + Snap->Slab = std::move(Slab).build(); for (auto &Iter : Snap->Slab) - Snap->Pointers.push_back(&Iter.second); + Snap->Pointers.push_back(&Iter); auto S = std::shared_ptr<std::vector<const Symbol *>>(std::move(Snap), &Snap->Pointers); I->build(std::move(S)); diff --git a/unittests/clangd/FileIndexTests.cpp b/unittests/clangd/FileIndexTests.cpp index c7bce7b5..d0c32d17 100644 --- a/unittests/clangd/FileIndexTests.cpp +++ b/unittests/clangd/FileIndexTests.cpp @@ -28,9 +28,11 @@ Symbol symbol(llvm::StringRef ID) { return Sym; } -void addNumSymbolsToSlab(int Begin, int End, SymbolSlab *Slab) { +std::unique_ptr<SymbolSlab> numSlab(int Begin, int End) { + SymbolSlab::Builder Slab; for (int i = Begin; i <= End; i++) - Slab->insert(symbol(std::to_string(i))); + Slab.insert(symbol(std::to_string(i))); + return llvm::make_unique<SymbolSlab>(std::move(Slab).build()); } std::vector<std::string> @@ -45,28 +47,15 @@ TEST(FileSymbolsTest, UpdateAndGet) { FileSymbols FS; EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre()); - auto Slab = llvm::make_unique<SymbolSlab>(); - addNumSymbolsToSlab(1, 3, Slab.get()); - - FS.update("f1", std::move(Slab)); - + FS.update("f1", numSlab(1, 3)); EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre("1", "2", "3")); } TEST(FileSymbolsTest, Overlap) { FileSymbols FS; - - auto Slab = llvm::make_unique<SymbolSlab>(); - addNumSymbolsToSlab(1, 3, Slab.get()); - - FS.update("f1", std::move(Slab)); - - Slab = llvm::make_unique<SymbolSlab>(); - addNumSymbolsToSlab(3, 5, Slab.get()); - - FS.update("f2", std::move(Slab)); - + FS.update("f1", numSlab(1, 3)); + FS.update("f2", numSlab(3, 5)); EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre("1", "2", "3", "3", "4", "5")); } @@ -74,17 +63,13 @@ TEST(FileSymbolsTest, Overlap) { TEST(FileSymbolsTest, SnapshotAliveAfterRemove) { FileSymbols FS; - auto Slab = llvm::make_unique<SymbolSlab>(); - addNumSymbolsToSlab(1, 3, Slab.get()); - - FS.update("f1", std::move(Slab)); + FS.update("f1", numSlab(1, 3)); auto Symbols = FS.allSymbols(); EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3")); FS.update("f1", nullptr); EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre()); - EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3")); } diff --git a/unittests/clangd/IndexTests.cpp b/unittests/clangd/IndexTests.cpp index 0a4f31a7..fac2895e 100644 --- a/unittests/clangd/IndexTests.cpp +++ b/unittests/clangd/IndexTests.cpp @@ -13,10 +13,10 @@ #include "gtest/gtest.h" using testing::UnorderedElementsAre; +using testing::Pointee; namespace clang { namespace clangd { - namespace { Symbol symbol(llvm::StringRef QName) { @@ -33,6 +33,24 @@ Symbol symbol(llvm::StringRef QName) { return Sym; } +MATCHER_P(Named, N, "") { return arg.Name == N; } + +TEST(SymbolSlab, FindAndIterate) { + SymbolSlab::Builder B; + B.insert(symbol("Z")); + B.insert(symbol("Y")); + B.insert(symbol("X")); + EXPECT_EQ(nullptr, B.find(SymbolID("W"))); + for (const char *Sym : {"X", "Y", "Z"}) + EXPECT_THAT(B.find(SymbolID(Sym)), Pointee(Named(Sym))); + + SymbolSlab S = std::move(B).build(); + EXPECT_THAT(S, UnorderedElementsAre(Named("X"), Named("Y"), Named("Z"))); + EXPECT_EQ(S.end(), S.find(SymbolID("W"))); + for (const char *Sym : {"X", "Y", "Z"}) + EXPECT_THAT(*S.find(SymbolID(Sym)), Named(Sym)); +} + struct SlabAndPointers { SymbolSlab Slab; std::vector<const Symbol *> Pointers; @@ -45,18 +63,18 @@ struct SlabAndPointers { std::shared_ptr<std::vector<const Symbol *>> generateSymbols(std::vector<std::string> QualifiedNames, std::weak_ptr<SlabAndPointers> *WeakSymbols = nullptr) { - auto Slab = std::make_shared<SlabAndPointers>(); - if (WeakSymbols) - *WeakSymbols = Slab; - + SymbolSlab::Builder Slab; for (llvm::StringRef QName : QualifiedNames) - Slab->Slab.insert(symbol(QName)); - - for (const auto &Sym : Slab->Slab) - Slab->Pointers.push_back(&Sym.second); + Slab.insert(symbol(QName)); - auto *Pointers = &Slab->Pointers; - return {std::move(Slab), Pointers}; + 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}; } // Create a slab of symbols with IDs and names [Begin, End], otherwise identical diff --git a/unittests/clangd/SymbolCollectorTests.cpp b/unittests/clangd/SymbolCollectorTests.cpp index acbecbae..1a6dc970 100644 --- a/unittests/clangd/SymbolCollectorTests.cpp +++ b/unittests/clangd/SymbolCollectorTests.cpp @@ -32,8 +32,7 @@ using testing::UnorderedElementsAre; // GMock helpers for matching Symbol. MATCHER_P(QName, Name, "") { - return (arg.second.Scope + (arg.second.Scope.empty() ? "" : "::") + - arg.second.Name).str() == Name; + return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name; } namespace clang { |