kbobyrev updated this revision to Diff 164369.
kbobyrev marked 9 inline comments as done.
kbobyrev added a comment.
Address a round of comments.
https://reviews.llvm.org/D51539
Files:
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/Index.cpp
clang-tools-extra/clangd/index/Index.h
clang-tools-extra/clangd/index/MemIndex.cpp
clang-tools-extra/clangd/index/MemIndex.h
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clangd/index/dex/DexIndex.h
clang-tools-extra/unittests/clangd/IndexTests.cpp
Index: clang-tools-extra/unittests/clangd/IndexTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/IndexTests.cpp
+++ clang-tools-extra/unittests/clangd/IndexTests.cpp
@@ -10,11 +10,11 @@
#include "Annotations.h"
#include "TestIndex.h"
#include "TestTU.h"
-#include "gmock/gmock.h"
#include "index/FileIndex.h"
#include "index/Index.h"
#include "index/MemIndex.h"
#include "index/Merge.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
using testing::_;
@@ -58,11 +58,11 @@
auto Token = std::make_shared<int>();
std::weak_ptr<int> WeakToken = Token;
- SwapIndex S(
- llvm::make_unique<MemIndex>(SymbolSlab(), RefSlab(), std::move(Token)));
- EXPECT_FALSE(WeakToken.expired()); // Current MemIndex keeps it alive.
+ SwapIndex S(llvm::make_unique<MemIndex>(
+ SymbolSlab(), RefSlab(), std::move(Token), /*BackingDataSize=*/0));
+ EXPECT_FALSE(WeakToken.expired()); // Current MemIndex keeps it alive.
S.reset(llvm::make_unique<MemIndex>()); // Now the MemIndex is destroyed.
- EXPECT_TRUE(WeakToken.expired()); // So the token is too.
+ EXPECT_TRUE(WeakToken.expired()); // So the token is too.
}
TEST(MemIndexTest, MemIndexDeduplicate) {
@@ -281,7 +281,7 @@
FileURI("unittest:///test2.cc"))))));
}
-MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
}
Index: clang-tools-extra/clangd/index/dex/DexIndex.h
===================================================================
--- clang-tools-extra/clangd/index/dex/DexIndex.h
+++ clang-tools-extra/clangd/index/dex/DexIndex.h
@@ -55,17 +55,21 @@
}
// Symbols are owned by BackingData, Index takes ownership.
template <typename Range, typename Payload>
- DexIndex(Range &&Symbols, Payload &&BackingData,
+ DexIndex(Range &&Symbols, Payload &&BackingData, size_t BackingDataSize,
llvm::ArrayRef<std::string> URISchemes)
: DexIndex(std::forward<Range>(Symbols), URISchemes) {
KeepAlive = std::shared_ptr<void>(
std::make_shared<Payload>(std::move(BackingData)), nullptr);
+ this->BackingDataSize = BackingDataSize;
}
/// Builds an index from a slab. The index takes ownership of the slab.
static std::unique_ptr<SymbolIndex>
build(SymbolSlab Slab, llvm::ArrayRef<std::string> URISchemes) {
- return llvm::make_unique<DexIndex>(Slab, std::move(Slab), URISchemes);
+ // Store Slab size before it is moved.
+ const auto BackingDataSize = Slab.bytes();
+ return llvm::make_unique<DexIndex>(Slab, std::move(Slab), BackingDataSize,
+ URISchemes);
}
bool
@@ -96,6 +100,8 @@
/// during the fuzzyFind process.
llvm::DenseMap<Token, PostingList> InvertedIndex;
std::shared_ptr<void> KeepAlive; // poor man's move-only std::any
+ // Size of memory retained by KeepAlive.
+ size_t BackingDataSize = 0;
std::vector<std::string> URISchemes;
};
Index: clang-tools-extra/clangd/index/dex/DexIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/dex/DexIndex.cpp
+++ clang-tools-extra/clangd/index/dex/DexIndex.cpp
@@ -228,15 +228,13 @@
}
size_t DexIndex::estimateMemoryUsage() const {
- size_t Bytes =
- LookupTable.size() * sizeof(std::pair<SymbolID, const Symbol *>);
- Bytes += SymbolQuality.size() * sizeof(std::pair<const Symbol *, float>);
- Bytes += InvertedIndex.size() * sizeof(Token);
-
- for (const auto &P : InvertedIndex) {
+ size_t Bytes = Symbols.size() * sizeof(const Symbol *);
+ Bytes += SymbolQuality.size() * sizeof(float);
+ Bytes += LookupTable.getMemorySize();
+ Bytes += InvertedIndex.getMemorySize();
+ for (const auto &P : InvertedIndex)
Bytes += P.second.size() * sizeof(DocID);
- }
- return Bytes;
+ return Bytes + BackingDataSize;
}
std::vector<std::string> generateProximityURIs(llvm::StringRef URIPath) {
Index: clang-tools-extra/clangd/index/MemIndex.h
===================================================================
--- clang-tools-extra/clangd/index/MemIndex.h
+++ clang-tools-extra/clangd/index/MemIndex.h
@@ -30,11 +30,13 @@
}
// Symbols are owned by BackingData, Index takes ownership.
template <typename SymbolRange, typename RefRange, typename Payload>
- MemIndex(SymbolRange &&Symbols, RefRange &&Refs, Payload &&BackingData)
+ MemIndex(SymbolRange &&Symbols, RefRange &&Refs, Payload &&BackingData,
+ size_t BackingDataSize)
: MemIndex(std::forward<SymbolRange>(Symbols),
std::forward<RefRange>(Refs)) {
KeepAlive = std::shared_ptr<void>(
std::make_shared<Payload>(std::move(BackingData)), nullptr);
+ this->BackingDataSize = BackingDataSize;
}
/// Builds an index from slabs. The index takes ownership of the data.
@@ -58,6 +60,8 @@
// A map from symbol ID to symbol refs, support query by IDs.
llvm::DenseMap<SymbolID, llvm::ArrayRef<Ref>> Refs;
std::shared_ptr<void> KeepAlive; // poor man's move-only std::any
+ // Size of memory retained by KeepAlive.
+ size_t BackingDataSize = 0;
};
} // namespace clangd
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -16,8 +16,11 @@
namespace clangd {
std::unique_ptr<SymbolIndex> MemIndex::build(SymbolSlab Slab, RefSlab Refs) {
+ // Store Slab size before it is moved.
+ const auto BackingDataSize = Slab.bytes();
auto Data = std::make_pair(std::move(Slab), std::move(Refs));
- return llvm::make_unique<MemIndex>(Data.first, Data.second, std::move(Data));
+ return llvm::make_unique<MemIndex>(Data.first, Data.second, std::move(Data),
+ BackingDataSize);
}
bool MemIndex::fuzzyFind(
@@ -70,7 +73,7 @@
}
size_t MemIndex::estimateMemoryUsage() const {
- return Index.getMemorySize();
+ return Index.getMemorySize() + BackingDataSize;
}
} // namespace clangd
Index: clang-tools-extra/clangd/index/Index.h
===================================================================
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -509,10 +509,13 @@
// 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 refs(const RefsRequest &,
llvm::function_ref<void(const Ref &)>) const override;
+
size_t estimateMemoryUsage() const override;
private:
Index: clang-tools-extra/clangd/index/Index.cpp
===================================================================
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -179,14 +179,17 @@
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::refs(const RefsRequest &R,
llvm::function_ref<void(const Ref &)> CB) const {
return snapshot()->refs(R, CB);
}
+
size_t SwapIndex::estimateMemoryUsage() const {
return snapshot()->estimateMemoryUsage();
}
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -131,7 +131,8 @@
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)));
+ std::move(RefsStorage)),
+ /*BackingDataSize=*/0);
}
void FileIndex::update(PathRef Path, ASTContext *AST,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
