[clang-tools-extra] r341800 - [clangd] Add symbol slab size to index memory consumption estimates

Kirill Bobyrev via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 10 04:46:07 PDT 2018


Author: omtcyfz
Date: Mon Sep 10 04:46:07 2018
New Revision: 341800

URL: http://llvm.org/viewvc/llvm-project?rev=341800&view=rev
Log:
[clangd] Add symbol slab size to index memory consumption estimates

Currently, `SymbolIndex::estimateMemoryUsage()` returns the "overhead"
estimate, i.e. the estimate of the Index data structure excluding
backing data (such as Symbol Slab and Reference Slab). This patch
propagates information about paired data size where necessary.

Reviewed By: ioeric, sammccall

Differential Revision: https://reviews.llvm.org/D51539

Modified:
    clang-tools-extra/trunk/clangd/index/FileIndex.cpp
    clang-tools-extra/trunk/clangd/index/MemIndex.cpp
    clang-tools-extra/trunk/clangd/index/MemIndex.h
    clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
    clang-tools-extra/trunk/clangd/index/dex/Dex.h
    clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp

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=341800&r1=341799&r2=341800&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Mon Sep 10 04:46:07 2018
@@ -127,11 +127,18 @@ std::unique_ptr<SymbolIndex> FileSymbols
     }
   }
 
+  size_t StorageSize = RefsStorage.size() * sizeof(Ref);
+  for (const auto &Slab : SymbolSlabs)
+    StorageSize += Slab->bytes();
+  for (const auto &RefSlab : RefSlabs)
+    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)));
+                      std::move(RefsStorage)),
+      StorageSize);
 }
 
 void FileIndex::update(PathRef Path, ASTContext *AST,

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=341800&r1=341799&r2=341800&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Mon Sep 10 04:46:07 2018
@@ -16,8 +16,11 @@ namespace clang {
 namespace clangd {
 
 std::unique_ptr<SymbolIndex> MemIndex::build(SymbolSlab Slab, RefSlab Refs) {
+  // Store Slab size before it is moved.
+  const auto BackingDataSize = Slab.bytes() + Refs.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 @@ void MemIndex::refs(const RefsRequest &R
 }
 
 size_t MemIndex::estimateMemoryUsage() const {
-  return Index.getMemorySize();
+  return Index.getMemorySize() + Refs.getMemorySize() + BackingDataSize;
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.h?rev=341800&r1=341799&r2=341800&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/MemIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.h Mon Sep 10 04:46:07 2018
@@ -30,11 +30,13 @@ public:
   }
   // 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 @@ private:
   // 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

Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=341800&r1=341799&r2=341800&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Mon Sep 10 04:46:07 2018
@@ -227,15 +227,13 @@ void Dex::refs(const RefsRequest &Req,
 }
 
 size_t Dex::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) {

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=341800&r1=341799&r2=341800&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Dex.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.h Mon Sep 10 04:46:07 2018
@@ -55,17 +55,21 @@ public:
   }
   // Symbols are owned by BackingData, Index takes ownership.
   template <typename Range, typename Payload>
-  Dex(Range &&Symbols, Payload &&BackingData,
+  Dex(Range &&Symbols, Payload &&BackingData, size_t BackingDataSize,
       llvm::ArrayRef<std::string> URISchemes)
       : Dex(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<Dex>(Slab, std::move(Slab), URISchemes);
+    // Store Slab size before it is moved.
+    const auto BackingDataSize = Slab.bytes();
+    return llvm::make_unique<Dex>(Slab, std::move(Slab), BackingDataSize,
+                                  URISchemes);
   }
 
   bool
@@ -96,6 +100,8 @@ private:
   /// 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;
 };

Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp?rev=341800&r1=341799&r2=341800&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Mon Sep 10 04:46:07 2018
@@ -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 @@ TEST(SwapIndexTest, OldIndexRecycled) {
   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) {




More information about the cfe-commits mailing list