kbobyrev updated this revision to Diff 165440.
kbobyrev added a comment.

Move `vlog` message to the outer `build(...)` function: otherwise 
`BackingMemorySize` is not set to the correct value and log reports index 
overhead (instead of the complete index + slab) size.


https://reviews.llvm.org/D52047

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h

Index: clang-tools-extra/clangd/index/dex/Dex.h
===================================================================
--- clang-tools-extra/clangd/index/dex/Dex.h
+++ clang-tools-extra/clangd/index/dex/Dex.h
@@ -21,6 +21,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_DEX_DEX_H
 
 #include "Iterator.h"
+#include "Logger.h"
 #include "Token.h"
 #include "Trigram.h"
 #include "index/Index.h"
@@ -53,6 +54,7 @@
       this->Symbols.push_back(&Sym);
     buildIndex();
   }
+
   // Symbols are owned by BackingData, Index takes ownership.
   template <typename Range, typename Payload>
   Dex(Range &&Symbols, Payload &&BackingData, size_t BackingDataSize,
@@ -68,8 +70,11 @@
   build(SymbolSlab Slab, llvm::ArrayRef<std::string> URISchemes) {
     // Store Slab size before it is moved.
     const auto BackingDataSize = Slab.bytes();
-    return llvm::make_unique<Dex>(Slab, std::move(Slab), BackingDataSize,
-                                  URISchemes);
+    auto Index = llvm::make_unique<Dex>(Slab, std::move(Slab), BackingDataSize,
+                                        URISchemes);
+    vlog("Built Dex with estimated memory usage {0} bytes.",
+         Index->estimateMemoryUsage());
+    return Index;
   }
 
   bool
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
@@ -10,7 +10,6 @@
 #include "Dex.h"
 #include "FileDistance.h"
 #include "FuzzyMatch.h"
-#include "Logger.h"
 #include "Quality.h"
 #include "llvm/ADT/StringSet.h"
 #include <algorithm>
@@ -118,9 +117,6 @@
     for (const auto &Token : generateSearchTokens(*Sym))
       InvertedIndex[Token].push_back(SymbolRank);
   }
-
-  vlog("Built Dex with estimated memory usage {0} bytes.",
-       estimateMemoryUsage());
 }
 
 /// Constructs iterators over tokens extracted from the query and exhausts it
@@ -232,8 +228,10 @@
   Bytes += SymbolQuality.size() * sizeof(float);
   Bytes += LookupTable.getMemorySize();
   Bytes += InvertedIndex.getMemorySize();
-  for (const auto &P : InvertedIndex)
+  for (const auto &P : InvertedIndex) {
+    Bytes += P.first.Data.size();
     Bytes += P.second.size() * sizeof(DocID);
+  }
   return Bytes + BackingDataSize;
 }
 
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===================================================================
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -66,23 +66,44 @@
   return Requests;
 }
 
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead?
+static void MemSize(benchmark::State &State) {
+  const auto Mem = buildMem();
+  for (auto _ : State)
+    // Divide size of Mem by 1000 so that it will be correctly displayed in the
+    // benchmark report (possible options for time units are ms, ns and us).
+    State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() /
+                           1000);
+}
+BENCHMARK(MemSize)->UseManualTime()->Unit(benchmark::kMillisecond);
+
+static void DexSize(benchmark::State &State) {
+  const auto Dex = buildDex();
+  for (auto _ : State)
+    State.SetIterationTime(Dex->estimateMemoryUsage() / 1000);
+}
+BENCHMARK(DexSize)->UseManualTime()->Unit(benchmark::kMillisecond);
+
 static void MemQueries(benchmark::State &State) {
   const auto Mem = buildMem();
   const auto Requests = extractQueriesFromLogs();
   for (auto _ : State)
     for (const auto &Request : Requests)
       Mem->fuzzyFind(Request, [](const Symbol &S) {});
 }
-BENCHMARK(MemQueries);
+BENCHMARK(MemQueries)->Unit(benchmark::kMillisecond);
 
 static void DexQueries(benchmark::State &State) {
   const auto Dex = buildDex();
   const auto Requests = extractQueriesFromLogs();
   for (auto _ : State)
     for (const auto &Request : Requests)
       Dex->fuzzyFind(Request, [](const Symbol &S) {});
 }
-BENCHMARK(DexQueries);
+BENCHMARK(DexQueries)->Unit(benchmark::kMillisecond);
 
 } // namespace
 } // namespace clangd
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to