[PATCH] D52503: [clangd] More precise index memory usage estimate

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 167062.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.

- Prevent `sizeof(std::vector<Chunk>)` to be counted twice in the memory estimates
- Pull memory usage logging to the caller side: it is currently incorrect because `Dex::BackingDataSize` is not set by the time `vlog(..., estimateMemoryUsage())` is called and hence the log only includes size of the index overhead

I will remove these changes from https://reviews.llvm.org/D52047 and hopefully make it easier to review.


https://reviews.llvm.org/D52503

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/PostingList.h


Index: clang-tools-extra/clangd/index/dex/PostingList.h
===================================================================
--- clang-tools-extra/clangd/index/dex/PostingList.h
+++ clang-tools-extra/clangd/index/dex/PostingList.h
@@ -66,10 +66,8 @@
   /// go through the chunks and decompress them on-the-fly when necessary.
   std::unique_ptr<Iterator> iterator() const;
 
-  /// Returns in-memory size.
-  size_t bytes() const {
-    return sizeof(Chunk) + Chunks.capacity() * sizeof(Chunk);
-  }
+  /// Returns in-memory size of underlying storage.
+  size_t bytes() const { return Chunks.capacity() * sizeof(Chunk); }
 
 private:
   const std::vector<Chunk> Chunks;
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===================================================================
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -130,9 +130,6 @@
   for (const auto &TokenToPostingList : TempInvertedIndex)
     InvertedIndex.insert(
         {TokenToPostingList.first, PostingList(TokenToPostingList.second)});
-
-  vlog("Built Dex with estimated memory usage {0} bytes.",
-       estimateMemoryUsage());
 }
 
 /// Constructs iterators over tokens extracted from the query and exhausts it
@@ -248,8 +245,9 @@
   Bytes += SymbolQuality.size() * sizeof(float);
   Bytes += LookupTable.getMemorySize();
   Bytes += InvertedIndex.getMemorySize();
-  for (const auto &P : InvertedIndex)
-    Bytes += P.second.bytes();
+  for (const auto &TokenToPostingList : InvertedIndex)
+    Bytes += TokenToPostingList.first.Data.size() +
+             TokenToPostingList.second.bytes();
   return Bytes + BackingDataSize;
 }
 
Index: clang-tools-extra/clangd/index/Serialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -6,8 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
+
 #include "Serialization.h"
 #include "Index.h"
+#include "Logger.h"
 #include "RIFF.h"
 #include "Trace.h"
 #include "dex/Dex.h"
@@ -433,8 +435,12 @@
   }
 
   trace::Span Tracer("BuildIndex");
-  return UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
-                : MemIndex::build(std::move(Symbols), std::move(Refs));
+  auto Index = UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
+                      : MemIndex::build(std::move(Symbols), std::move(Refs));
+  vlog("Loaded {0} from {1} with estimated memory usage {2}",
+       UseDex ? "Dex" : "MemIndex", SymbolFilename,
+       Index->estimateMemoryUsage());
+  return Index;
 }
 
 } // namespace clangd


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52503.167062.patch
Type: text/x-patch
Size: 2732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180926/8ddfffc3/attachment.bin>