[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking

[Kirill Bobyrev via Phabricator via cfe-commits]

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