kbobyrev added inline comments.

================
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:81
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead?
----------------
ilya-biryukov wrote:
> Given the trick we use for display, how are we going to show **two** memory 
> uses?
As discussed offline, this hack also relies on the fact that benchmark has a 
dynamic nature of determining iterations count. Giving a large number of "time 
units" to the  counter results in a single iteration.

I've tried to understand whether I could use any flags for [[ 
https://github.com/google/benchmark#user-defined-counters | User-Defined 
Counter ]] that could just divide the number of iterations by `IterationTime`, 
but I could find one that would do exactly what is needed here. Also, I didn't 
find any way to manually set the iteration count.


================
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:239
   for (const auto &P : InvertedIndex)
-    Bytes += P.second.bytes();
+    Bytes += P.first.Data.size() + P.second.bytes();
   return Bytes + BackingDataSize;
----------------
ilya-biryukov wrote:
> Just to clarify: `P.first.Data.size()` is the size of the arena for all the 
> symbols?
`P` is `std::pair<Token, PostingList>` here, so that would be 
`Token.Data.size()` which is the size of `std::string` stored in Token (e.g. 
trigram symbols for `Trigram` tokens, directory URI for `ProximityPath` tokens, 
etc). `P` is probably bad here, I'll change the naming to be more explicit.


https://reviews.llvm.org/D52047



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to