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

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 21 08:12:34 PDT 2018


ioeric added inline comments.


================
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:78
 
+static void DexQueries(benchmark::State &State) {
+  const auto Dex = buildDex();
----------------
Why did we move this benchmark (`DexQueries`)? It seems unnecessary.


================
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:106
+    // benchmark report (possible options for time units are ms, ns and us).
+    State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() /
+                           1000);
----------------
nit: maybe divide by `1000.0` to avoid precision loss?


================
Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:187
 
+llvm::Expected<SymbolSlab> symbolsFromFile(llvm::StringRef SymbolFilename) {
+  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
----------------
Could you put this near the old `loadIndex` to preserve the revision history?


https://reviews.llvm.org/D52047





More information about the cfe-commits mailing list