ioeric added inline comments.
================ Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:144 +size_t FileIndex::estimateMemoryUsage() const { + return FSymbols.estimateMemoryUsage(); +} ---------------- sammccall wrote: > ioeric wrote: > > 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`. > (I had trouble parsing this at first - If I understand right, you want the > *memIndex* to own the size, and FileIndex to just inherit > SwapIndex::estimateMemoryUsage which delegates to > MemIndex::estimateMemoryUsage?) > > I like this idea, it's simpler and also means one less place you need to > lock, which is always nice. Yes, that's correct. Thanks for the clarification! https://reviews.llvm.org/D51539 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits