[PATCH] D52503: [clangd] Fix bugs with incorrect memory estimate report

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 167079.
kbobyrev retitled this revision from "[clangd] More precise index memory usage estimate" to "[clangd] Fix bugs with incorrect memory estimate report".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.

As discussed offline, using `std::string::size()` would not be a precise estimate of `std::string` size, therefore I'm excluding `Token::Data` size estimate.


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 external 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,8 @@
   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.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.167079.patch
Type: text/x-patch
Size: 2677 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180926/6cea3188/attachment-0001.bin>