ioeric added inline comments.
================ Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:144 +size_t FileIndex::estimateMemoryUsage() const { + return FSymbols.estimateMemoryUsage(); +} ---------------- This can be a bit tricky. Generally, the size of a `FileIndex` is the size of FSymbols plus the overhead of the underlying index, but there can be multiple snapshots of symbols in `SwapIndex`. I think we can treat them as burst and only consider the size of the latest snapshot. `FSymbols` and the index have shared ownership to the symbol corpus. We can make either of them bookkeep the size, but I think it might be more straightforward to let the index "own" the size. You could set the payload size when creating the index, and you wouldn't need to expose `estimateMemoryUsage` from `FSymbols`. ================ Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:44 + DexIndex(Range &&Symbols, llvm::ArrayRef<std::string> URISchemes, + size_t BackingMemory = 0) + : BackingMemory(BackingMemory), URISchemes(URISchemes) { ---------------- For this constructor, the index doesn't own the backing data. We should probably not include it in the estimation? (same for MemIndex) ================ Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:53 DexIndex(Range &&Symbols, Payload &&BackingData, - llvm::ArrayRef<std::string> URISchemes) - : DexIndex(std::forward<Range>(Symbols), URISchemes) { + llvm::ArrayRef<std::string> URISchemes, size_t BackingMemory = 0) + : DexIndex(std::forward<Range>(Symbols), URISchemes, BackingMemory) { ---------------- nit: maybe call `BackingDataSize`? `BackingMemory` sounds like the actual memory. Maybe also put the parameter right after `BackingData`, as they are closely related? (same for MemIndex) ================ Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:100 + /// Slab. + size_t BackingMemory; ---------------- nit: `=0`? https://reviews.llvm.org/D51539 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits