ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land.
lg ================ Comment at: clangd/ClangdUnit.cpp:664 + std::lock_guard<std::mutex> Lock(Mutex); + return ASTMemUsage + PreambleMemUsage; +} ---------------- ilya-biryukov wrote: > ioeric wrote: > > Would it be possible to calculate the usages lazily on request, so that you > > don't need to maintain two states? > It's really complicated since we store `std::future`s. This is one of the > caveats that I'm trying to fix with threading revamp, implementation will > change accordingly. For now, having two states is much easier. Thanks for the explanation! A `FIXME` would be appropriate here then? In general, I wouldn't expect this API to be called very often. ================ Comment at: unittests/clangd/ClangdTests.cpp:428 +TEST_F(ClangdVFSTest, MemoryUsage) { + MockFSProvider FS; ---------------- ilya-biryukov wrote: > ioeric wrote: > > Can we add a test for non-empty files and check that the usage is > > reasonable (e.g. `>0`)? > We do exactly that. My bad! ``` struct Something { int method(); }; ``` looks like real code on phabricator! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42480 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits