[clang-tools-extra] b8c3715 - [clangd] Trim memory periodically when using glibc malloc
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 21 23:54:48 PST 2020
Author: Quentin Chateau
Date: 2020-12-22T08:54:28+01:00
New Revision: b8c37153d5393aad96feefe0b4689b7b62bc160d
URL: https://github.com/llvm/llvm-project/commit/b8c37153d5393aad96feefe0b4689b7b62bc160d
DIFF: https://github.com/llvm/llvm-project/commit/b8c37153d5393aad96feefe0b4689b7b62bc160d.diff
LOG: [clangd] Trim memory periodically when using glibc malloc
This diff addresses the issue of the ever increasing memory usage of clangd. The key to understand what happens is to use `malloc_stats()`: malloc arenas keep getting bigger, although the actual memory used does not. It seems some operations while bulding the indices (both dynamic and background) create this problem. Specifically, 'FileSymbols::update' and 'FileSymbols::buildIndex' seem especially affected.
This diff adds a call to `malloc_trim()` periodically in
ClangdLSPServer.
Fixes: https://github.com/clangd/clangd/issues/251
Fixes: https://github.com/clangd/clangd/issues/115
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D93452
Added:
Modified:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/Features.inc.in
clang-tools-extra/clangd/tool/ClangdMain.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt
index 919457f216c1..9e62e0948027 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -14,9 +14,12 @@ if (NOT DEFINED CLANGD_BUILD_XPC)
unset(CLANGD_BUILD_XPC_DEFAULT)
endif ()
+option(CLANGD_MALLOC_TRIM "Call malloc_trim(3) periodically in Clangd. (only takes effect when using glibc)" ON)
+
llvm_canonicalize_cmake_booleans(
CLANGD_BUILD_XPC
CLANGD_ENABLE_REMOTE
+ CLANGD_MALLOC_TRIM
LLVM_ENABLE_ZLIB
)
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index b32c9e13973b..0c42f95fb594 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -178,6 +178,7 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
} else if (auto Handler = Notifications.lookup(Method)) {
Handler(std::move(Params));
Server.maybeExportMemoryProfile();
+ Server.maybeCleanupMemory();
} else {
log("unhandled notification {0}", Method);
}
@@ -453,6 +454,7 @@ void ClangdLSPServer::callRaw(StringRef Method, llvm::json::Value Params,
void ClangdLSPServer::notify(llvm::StringRef Method, llvm::json::Value Params) {
log("--> {0}", Method);
+ maybeCleanupMemory();
std::lock_guard<std::mutex> Lock(TranspWriter);
Transp.notify(Method, std::move(Params));
}
@@ -1301,6 +1303,27 @@ void ClangdLSPServer::maybeExportMemoryProfile() {
NextProfileTime = Now + ProfileInterval;
}
+void ClangdLSPServer::maybeCleanupMemory() {
+ // Memory cleanup is probably expensive, throttle it
+ static constexpr auto MemoryCleanupInterval = std::chrono::minutes(1);
+
+ if (!Opts.MemoryCleanup)
+ return;
+
+ // FIXME: this can probably be done without a mutex
+ // and the logic could be shared with maybeExportMemoryProfile
+ {
+ auto Now = std::chrono::steady_clock::now();
+ std::lock_guard<std::mutex> Lock(NextMemoryCleanupTimeMutex);
+ if (Now < NextMemoryCleanupTime)
+ return;
+ NextMemoryCleanupTime = Now + MemoryCleanupInterval;
+ }
+
+ vlog("Calling memory cleanup callback");
+ Opts.MemoryCleanup();
+}
+
// FIXME: This function needs to be properly tested.
void ClangdLSPServer::onChangeConfiguration(
const DidChangeConfigurationParams &Params) {
@@ -1507,8 +1530,9 @@ ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
MsgHandler->bind("textDocument/foldingRange", &ClangdLSPServer::onFoldingRange);
// clang-format on
- // Delay first profile until we've finished warming up.
- NextProfileTime = std::chrono::steady_clock::now() + std::chrono::minutes(1);
+ // Delay first profile and memory cleanup until we've finished warming up.
+ NextMemoryCleanupTime = NextProfileTime =
+ std::chrono::steady_clock::now() + std::chrono::minutes(1);
}
ClangdLSPServer::~ClangdLSPServer() {
@@ -1621,6 +1645,10 @@ void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
void ClangdLSPServer::onBackgroundIndexProgress(
const BackgroundQueue::Stats &Stats) {
static const char ProgressToken[] = "backgroundIndexProgress";
+
+ // The background index did some work, maybe we need to cleanup
+ maybeCleanupMemory();
+
std::lock_guard<std::mutex> Lock(BackgroundIndexProgressMutex);
auto NotifyProgress = [this](const BackgroundQueue::Stats &Stats) {
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index e65fc0e8a006..b5f9d2c9d766 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -48,6 +48,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
llvm::Optional<Path> CompileCommandsDir;
/// The offset-encoding to use, or None to negotiate it over LSP.
llvm::Optional<OffsetEncoding> Encoding;
+ /// If set, periodically called to release memory.
+ /// Consider malloc_trim(3)
+ std::function<void()> MemoryCleanup = nullptr;
/// Per-feature options. Generally ClangdServer lets these vary
/// per-request, but LSP allows limited/no customizations.
@@ -184,10 +187,18 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
/// profiling hasn't happened recently.
void maybeExportMemoryProfile();
+ /// Run the MemoryCleanup callback if it's time.
+ /// This method is thread safe.
+ void maybeCleanupMemory();
+
/// Timepoint until which profiling is off. It is used to throttle profiling
/// requests.
std::chrono::steady_clock::time_point NextProfileTime;
+ /// Next time we want to call the MemoryCleanup callback.
+ std::mutex NextMemoryCleanupTimeMutex;
+ std::chrono::steady_clock::time_point NextMemoryCleanupTime;
+
/// Since initialization of CDBs and ClangdServer is done lazily, the
/// following context captures the one used while creating ClangdLSPServer and
/// passes it to above mentioned object instances to make sure they share the
diff --git a/clang-tools-extra/clangd/Features.inc.in b/clang-tools-extra/clangd/Features.inc.in
index 6797232ddac7..c21d2b145571 100644
--- a/clang-tools-extra/clangd/Features.inc.in
+++ b/clang-tools-extra/clangd/Features.inc.in
@@ -1,2 +1,3 @@
#define CLANGD_BUILD_XPC @CLANGD_BUILD_XPC@
#define CLANGD_ENABLE_REMOTE @CLANGD_ENABLE_REMOTE@
+#define CLANGD_MALLOC_TRIM @CLANGD_MALLOC_TRIM@
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 331241115302..d2c52cf61c53 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -50,6 +50,10 @@
#include <unistd.h>
#endif
+#ifdef __GLIBC__
+#include <malloc.h>
+#endif
+
namespace clang {
namespace clangd {
@@ -497,6 +501,29 @@ opt<bool> CollectMainFileRefs{
init(ClangdServer::Options().CollectMainFileRefs),
};
+#if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
+opt<bool> EnableMallocTrim{
+ "malloc-trim",
+ cat(Misc),
+ desc("Release memory periodically via malloc_trim(3)."),
+ init(true),
+};
+
+std::function<void()> getMemoryCleanupFunction() {
+ if (!EnableMallocTrim)
+ return nullptr;
+ // Leave a few MB at the top of the heap: it is insignificant
+ // and will most likely be needed by the main thread
+ constexpr size_t MallocTrimPad = 20'000'000;
+ return []() {
+ if (malloc_trim(MallocTrimPad))
+ vlog("Released memory via malloc_trim");
+ };
+}
+#else
+std::function<void()> getMemoryCleanupFunction() { return nullptr; }
+#endif
+
#if CLANGD_ENABLE_REMOTE
opt<std::string> RemoteIndexAddress{
"remote-index-address",
@@ -797,6 +824,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
Opts.BuildRecoveryAST = RecoveryAST;
Opts.PreserveRecoveryASTType = RecoveryASTType;
Opts.FoldingRanges = FoldingRanges;
+ Opts.MemoryCleanup = getMemoryCleanupFunction();
Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Opts.CodeComplete.Limit = LimitResults;
More information about the cfe-commits
mailing list