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

Reply via email to