kadircet updated this revision to Diff 286043.
kadircet marked an inline comment as done.
kadircet added a comment.
- Rename the overload
- Add comments around possible caveats that might result in inaccuracies.
- Move the metric recording itself into another thread.
- Keep the calculations in the main thread, as they seemed to have <1ms
latency, even with huge preambles/asts.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86077/new/
https://reviews.llvm.org/D86077
Files:
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/support/Trace.cpp
clang-tools-extra/clangd/support/Trace.h
Index: clang-tools-extra/clangd/support/Trace.h
===================================================================
--- clang-tools-extra/clangd/support/Trace.h
+++ clang-tools-extra/clangd/support/Trace.h
@@ -68,6 +68,10 @@
const llvm::StringLiteral LabelName;
};
+/// Convenient helper for collecting memory usage metrics from across multiple
+/// components. Results are recorded under a metric named "memory_usage".
+void recordMemoryUsage(double Value, llvm::StringRef ComponentName);
+
/// A consumer of trace events and measurements. The events are produced by
/// Spans and trace::log, the measurements are produced by Metrics::record.
/// Implementations of this interface must be thread-safe.
Index: clang-tools-extra/clangd/support/Trace.cpp
===================================================================
--- clang-tools-extra/clangd/support/Trace.cpp
+++ clang-tools-extra/clangd/support/Trace.cpp
@@ -326,6 +326,13 @@
Context EventTracer::beginSpan(llvm::StringRef Name, llvm::json::Object *Args) {
return Context::current().clone();
}
+
+void recordMemoryUsage(double Value, llvm::StringRef ComponentName) {
+ static constexpr Metric MemoryUsage("memory_usage", Metric::Value,
+ "component_name");
+
+ MemoryUsage.record(Value, ComponentName);
+}
} // namespace trace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -23,6 +23,7 @@
#include "index/dex/Dex.h"
#include "support/Logger.h"
#include "support/Path.h"
+#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
#include "clang/Index/IndexingAction.h"
#include "clang/Index/IndexingOptions.h"
@@ -399,6 +400,7 @@
auto NewIndex =
PreambleSymbols.buildIndex(UseDex ? IndexType::Heavy : IndexType::Light,
DuplicateHandling::PickOne, &IndexVersion);
+ trace::recordMemoryUsage(NewIndex->estimateMemoryUsage(), "preamble_index");
{
std::lock_guard<std::mutex> Lock(UpdateIndexMu);
if (IndexVersion <= PreambleIndexVersion) {
@@ -424,6 +426,7 @@
size_t IndexVersion = 0;
auto NewIndex = MainFileSymbols.buildIndex(
IndexType::Light, DuplicateHandling::Merge, &IndexVersion);
+ trace::recordMemoryUsage(NewIndex->estimateMemoryUsage(), "main_file_index");
{
std::lock_guard<std::mutex> Lock(UpdateIndexMu);
if (IndexVersion <= MainIndexVersion) {
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -298,6 +298,11 @@
/// This class stores per-file data in the Files map.
struct FileData;
+ /// Records estimated memory usage for the TUScheduler internals. These are
+ /// best-effort estimates as TUScheduler itself is multi-threaded and internal
+ /// state is likely to change during the calculations.
+ void recordMemoryUsage();
+
public:
/// Responsible for retaining and rebuilding idle ASTs. An implementation is
/// an LRU cache.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -77,6 +77,7 @@
#include <algorithm>
#include <chrono>
#include <condition_variable>
+#include <cstddef>
#include <functional>
#include <memory>
#include <mutex>
@@ -164,6 +165,14 @@
return llvm::Optional<std::unique_ptr<ParsedAST>>(std::move(V));
}
+ size_t getTotalUsedBytes() {
+ size_t TotalBytes = 0;
+ std::lock_guard<std::mutex> Lock(Mut);
+ for (const auto &Elem : LRU)
+ TotalBytes += Elem.second->getUsedBytes();
+ return TotalBytes;
+ }
+
private:
using KVPair = std::pair<Key, std::unique_ptr<ParsedAST>>;
@@ -1272,6 +1281,7 @@
bool TUScheduler::update(PathRef File, ParseInputs Inputs,
WantDiagnostics WantDiags) {
+ recordMemoryUsage();
std::unique_ptr<FileData> &FD = Files[File];
bool NewFile = FD == nullptr;
if (!FD) {
@@ -1430,5 +1440,32 @@
return P;
}
+void TUScheduler::recordMemoryUsage() {
+ // We defer recording of the measurements to a worker thread, as it might
+ // perform IO and we don't want to block main thread. We perform the
+ // calculation itself here, as it accesses TUScheduler state, which is not
+ // threadsafe. It is cheap enough though.
+ size_t ASTCacheBytes = IdleASTs->getTotalUsedBytes();
+ size_t PreambleBytes = 0;
+ // Otherwise preambles are stored on disk and we only keep filename in memory.
+ if (Opts.StorePreamblesInMemory) {
+ for (const auto &Elem : fileStats())
+ PreambleBytes += Elem.second.UsedBytes;
+ // fileStats results include ast cache sizes too, subtract them. Note that
+ // the size of the ast cache seen by fileStats() and here might differ in
+ // some circumstances. E.g. user closes a file that still has some pending
+ // action (like preamble build), the cached ast size will be included in
+ // ASTCacheBytes, but not in fileStats() as file is no longer active. This
+ // might also overshoot, e.g. worker might cache an AST between the
+ // calculation of ASTCacheBytes and fileStats().
+ // We clamp to guarantee non-negative values.
+ PreambleBytes -= std::min(PreambleBytes, ASTCacheBytes);
+ }
+
+ run("RecordMemoryUsage", /*Path=*/"", [ASTCacheBytes, PreambleBytes] {
+ trace::recordMemoryUsage(ASTCacheBytes, "ast_cache");
+ trace::recordMemoryUsage(PreambleBytes, "preambles");
+ });
+}
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
