[llvm-branch-commits] [clang-tools-extra] b8c3715 - [clangd] Trim memory periodically when using glibc malloc

Sam McCall via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Dec 21 23:58:50 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 llvm-branch-commits mailing list