kbobyrev updated this revision to Diff 167267.
kbobyrev added a comment.
Pass data from I/O to `readIndexFile(StringRef)`.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
=
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:96
-static void DexQueries(benchmark::State &State) {
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
--
kbobyrev updated this revision to Diff 167067.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Don't include misc changes elsewhere: focus on adding more benchmarks in this
revision.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmar
sammccall requested changes to this revision.
sammccall added a comment.
This revision now requires changes to proceed.
This change does several things, and I think most of them need further thought.
Can we discuss Monday?
Comment at: clang-tools-extra/clangd/benchmarks/IndexB
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.
===
kbobyrev updated this revision to Diff 166481.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
- Be more explicit about the nature of "benchmarks" with memory tracking logic
via `State::SetLabel(...)`.
- Force single iteration for "benchmarks" with memory usage tracking
- Add
lebedev.ri 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) ove
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) overh
kbobyrev updated this revision to Diff 166462.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.
Use `llvm::Expected`, cleanup the patch.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/Serializa
ilya-biryukov added a comment.
Also not sure about the trick:
- Would be surprising to see the "ms" instead of "mbytes"
- Time tends to vary between runs, so using Google Benchmark's capabilities to
run multiple times and report aggregated times seems useful. Not so much for
the memory usage: i
kbobyrev updated this revision to Diff 166271.
kbobyrev added a comment.
Remove `BuildMem` benchmark, which collects data about `MemIndex` building time
(which is essentially nothing and therefore not really interesting).
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchm
kbobyrev updated this revision to Diff 166269.
kbobyrev added a comment.
Add benchmark for `SymbolSlab` loading from file. This might be useful for
RIFF/YAML symbol loader optimizations.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-too
kbobyrev updated this revision to Diff 166266.
kbobyrev retitled this revision from "[clangd] Add a "benchmark" for tracking
memory" to "[clangd] Add building benchmark and memory consumption tracking".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Add symbol index build
13 matches
Mail list logo
