From b8c37153d5393aad96feefe0b4689b7b62bc160d Mon Sep 17 00:00:00 2001 From: Quentin Chateau Date: Tue, 22 Dec 2020 08:44:20 +0100 Subject: [PATCH] [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 --- clang-tools-extra/clangd/CMakeLists.txt | 3 ++ clang-tools-extra/clangd/ClangdLSPServer.cpp | 32 ++++++++++++++++++-- clang-tools-extra/clangd/ClangdLSPServer.h | 11 +++++++ clang-tools-extra/clangd/Features.inc.in | 1 + clang-tools-extra/clangd/tool/ClangdMain.cpp | 28 +++++++++++++++++ 5 files changed, 73 insertions(+), 2 deletions(-) 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 @@ public: } 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 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 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 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 @@ public: llvm::Optional CompileCommandsDir; /// The offset-encoding to use, or None to negotiate it over LSP. llvm::Optional Encoding; + /// If set, periodically called to release memory. + /// Consider malloc_trim(3) + std::function MemoryCleanup = nullptr; /// Per-feature options. Generally ClangdServer lets these vary /// per-request, but LSP allows limited/no customizations. @@ -184,10 +187,18 @@ private: /// 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 #endif +#ifdef __GLIBC__ +#include +#endif + namespace clang { namespace clangd { @@ -497,6 +501,29 @@ opt CollectMainFileRefs{ init(ClangdServer::Options().CollectMainFileRefs), }; +#if defined(__GLIBC__) && CLANGD_MALLOC_TRIM +opt EnableMallocTrim{ + "malloc-trim", + cat(Misc), + desc("Release memory periodically via malloc_trim(3)."), + init(true), +}; + +std::function 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 getMemoryCleanupFunction() { return nullptr; } +#endif + #if CLANGD_ENABLE_REMOTE opt 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;