ioeric added inline comments.
================
Comment at: clangd/ClangdServer.h:324
+ /// Returns estimated memory usage for each of the currently files.
+ /// The order of results is unspecified.
----------------
s/currently/current/ ? or current open?
================
Comment at: clangd/ClangdServer.h:326
+ /// The order of results is unspecified.
+ std::vector<std::pair<Path, std::size_t>> getUsedBytesPerFile() const;
+
----------------
Is the total usage of all files a good estimate of the overall clangd memory
usage? It might be worth mentioning the relationship between this and overall
clangd memory here.
================
Comment at: clangd/ClangdUnit.cpp:664
+ std::lock_guard<std::mutex> Lock(Mutex);
+ return ASTMemUsage + PreambleMemUsage;
+}
----------------
Would it be possible to calculate the usages lazily on request, so that you
don't need to maintain two states?
================
Comment at: clangd/ClangdUnit.h:100
+ /// Returns the esitmated size of the AST and the accessory structures, in
+ /// bytes. Does not include the size of the preamble.
+ std::size_t getUsedBytes() const;
----------------
It seems that `CppFile::getUsedBytes()` includes preamble?
================
Comment at: unittests/clangd/ClangdTests.cpp:428
+TEST_F(ClangdVFSTest, MemoryUsage) {
+ MockFSProvider FS;
----------------
Can we add a test for non-empty files and check that the usage is reasonable
(e.g. `>0`)?
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
