adamcz added inline comments.
================ Comment at: clang-tools-extra/clangd/TUScheduler.cpp:172 + for (const auto &Elem : LRU) + TotalBytes += Elem.second->getUsedBytes(); + return TotalBytes; ---------------- Any idea how expensive this is? I suppose TUScheduler::update() is rare enough that it's not a big deal? I ask because of bad experience with estimating memory usage inside critical path on another project ;-) Maybe we can add a span around this, so we can see if it's expensive in traces? Or maybe that will turn out to be more expensive than memory estimation? ================ Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1299 + + // Update memory usage metrics. Note that these are just estimates. + size_t ASTCacheBytes = IdleASTs->getUsedBytes(); ---------------- Isn't FD->Worker->update() above async (with correct options)? If so, this is still reading the old data, right? ================ Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1311 + } + trace::recordMemoryUsage(PreambleBytes, "preambles"); return NewFile; ---------------- Would it make sense to collect the number of preambles and ASTs cached at this point as well? Exported under a different metric, but at the same time, to be able to join the data together? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86077/new/ https://reviews.llvm.org/D86077 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
