qchateau updated this revision to Diff 312781.
qchateau added a comment.
Herald added subscribers: llvm-commits, dexonsmith, hiraditya.
Herald added a project: LLVM.

- Add MallocTrim to llvm::sys::Process
- Implement malloc trimming in ClangdLSPServer


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93452/new/

https://reviews.llvm.org/D93452

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/include/llvm/Support/Process.h
  llvm/lib/Support/Unix/Process.inc
  llvm/lib/Support/Windows/Process.inc

Index: llvm/lib/Support/Windows/Process.inc
===================================================================
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -81,6 +81,11 @@
   return size;
 }
 
+bool Process::MallocTrim(size_t Pad) {
+#warning Cannot get malloc info on this platform
+  return false;
+}
+
 void Process::GetTimeUsage(TimePoint<> &elapsed, std::chrono::nanoseconds &user_time,
                            std::chrono::nanoseconds &sys_time) {
   elapsed = std::chrono::system_clock::now();;
Index: llvm/lib/Support/Unix/Process.inc
===================================================================
--- llvm/lib/Support/Unix/Process.inc
+++ llvm/lib/Support/Unix/Process.inc
@@ -31,7 +31,7 @@
 #if HAVE_SIGNAL_H
 #include <signal.h>
 #endif
-#if defined(HAVE_MALLINFO)
+#if defined(HAVE_MALLINFO) || defined(HAVE_MALLOC_TRIM)
 #include <malloc.h>
 #endif
 #if defined(HAVE_MALLCTL)
@@ -117,6 +117,15 @@
 #endif
 }
 
+bool Process::MallocTrim(size_t Pad) {
+#if defined(HAVE_MALLOC_TRIM)
+  return malloc_trim(Pad);
+#else
+#warning Cannot get malloc info on this platform
+  return false;
+#endif
+}
+
 void Process::GetTimeUsage(TimePoint<> &elapsed, std::chrono::nanoseconds &user_time,
                            std::chrono::nanoseconds &sys_time) {
   elapsed = std::chrono::system_clock::now();
Index: llvm/include/llvm/Support/Process.h
===================================================================
--- llvm/include/llvm/Support/Process.h
+++ llvm/include/llvm/Support/Process.h
@@ -75,6 +75,12 @@
   /// allocated space.
   static size_t GetMallocUsage();
 
+  /// Attempts to free unused memory from the heap.
+  /// This function is basically a call to malloc_trim(3).
+  /// \param Pad Free space to leave at the top of the heap
+  /// \returns true if memory was actually released
+  static bool MallocTrim(size_t Pad);
+
   /// This static function will set \p user_time to the amount of CPU time
   /// spent in user (non-kernel) mode and \p sys_time to the amount of CPU
   /// time spent in system (kernel) mode.  If the operating system does not
Index: llvm/include/llvm/Config/config.h.cmake
===================================================================
--- llvm/include/llvm/Config/config.h.cmake
+++ llvm/include/llvm/Config/config.h.cmake
@@ -136,6 +136,9 @@
 /* Define to 1 if you have the `mallinfo' function. */
 #cmakedefine HAVE_MALLINFO ${HAVE_MALLINFO}
 
+/* Define to 1 if you have the `mallinfo' function. */
+#cmakedefine HAVE_MALLOC_TRIM ${HAVE_MALLOC_TRIM}
+
 /* Define to 1 if you have the <malloc/malloc.h> header file. */
 #cmakedefine HAVE_MALLOC_MALLOC_H ${HAVE_MALLOC_MALLOC_H}
 
Index: llvm/cmake/config-ix.cmake
===================================================================
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -232,6 +232,7 @@
 set(CMAKE_REQUIRED_DEFINITIONS "")
 check_symbol_exists(mallctl malloc_np.h HAVE_MALLCTL)
 check_symbol_exists(mallinfo malloc.h HAVE_MALLINFO)
+check_symbol_exists(malloc_trim malloc.h HAVE_MALLOC_TRIM)
 check_symbol_exists(malloc_zone_statistics malloc/malloc.h
                     HAVE_MALLOC_ZONE_STATISTICS)
 check_symbol_exists(getrlimit "sys/types.h;sys/time.h;sys/resource.h" HAVE_GETRLIMIT)
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -497,6 +497,14 @@
     init(ClangdServer::Options().CollectMainFileRefs),
 };
 
+opt<unsigned> MemoryCleanupPeriod{
+    "memory-cleanup-period",
+    cat(Misc),
+    desc("Period at which clangd will actively try to perform a "
+         "memory cleanup (in seconds)."),
+    init(ClangdLSPServer::Options().MemoryCleanupPeriod.count()),
+};
+
 #if CLANGD_ENABLE_REMOTE
 opt<std::string> RemoteIndexAddress{
     "remote-index-address",
@@ -797,6 +805,7 @@
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.MemoryCleanupPeriod = std::chrono::seconds(MemoryCleanupPeriod);
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -32,6 +32,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -48,6 +48,8 @@
     llvm::Optional<Path> CompileCommandsDir;
     /// The offset-encoding to use, or None to negotiate it over LSP.
     llvm::Optional<OffsetEncoding> Encoding;
+    // MallocTrim minimal period
+    std::chrono::seconds MemoryCleanupPeriod = std::chrono::seconds(60);
 
     /// Per-feature options. Generally ClangdServer lets these vary
     /// per-request, but LSP allows limited/no customizations.
@@ -184,10 +186,19 @@
   /// profiling hasn't happened recently.
   void maybeExportMemoryProfile();
 
+  /// Run MallocTrim if it's time.
+  /// This method is thread safe.
+  void maybeMallocTrim();
+
   /// 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 MallocTrim.
+  std::mutex NextMallocTrimMutex;
+  std::chrono::steady_clock::time_point NextMallocTrim;
+  std::chrono::seconds MallocTrimPeriod;
+
   /// 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
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -36,6 +36,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
@@ -178,6 +179,7 @@
     } else if (auto Handler = Notifications.lookup(Method)) {
       Handler(std::move(Params));
       Server.maybeExportMemoryProfile();
+      Server.maybeMallocTrim();
     } else {
       log("unhandled notification {0}", Method);
     }
@@ -453,6 +455,7 @@
 
 void ClangdLSPServer::notify(llvm::StringRef Method, llvm::json::Value Params) {
   log("--> {0}", Method);
+  maybeMallocTrim();
   std::lock_guard<std::mutex> Lock(TranspWriter);
   Transp.notify(Method, std::move(Params));
 }
@@ -1295,6 +1298,23 @@
   NextProfileTime = Now + ProfileInterval;
 }
 
+void ClangdLSPServer::maybeMallocTrim() {
+  // Leave a few MB at the top of the heap: it is insignificant
+  // and will most likely be needed by the main thread
+  static constexpr size_t MallocTrimPad = 20'000'000;
+
+  {
+    auto Now = std::chrono::steady_clock::now();
+    std::lock_guard<std::mutex> Lock(NextMallocTrimMutex);
+    if (Now < NextMallocTrim)
+      return;
+    NextMallocTrim = Now + MallocTrimPeriod;
+  }
+
+  vlog("Trimming memory");
+  llvm::sys::Process::MallocTrim(MallocTrimPad);
+}
+
 // FIXME: This function needs to be properly tested.
 void ClangdLSPServer::onChangeConfiguration(
     const DidChangeConfigurationParams &Params) {
@@ -1503,6 +1523,9 @@
 
   // Delay first profile until we've finished warming up.
   NextProfileTime = std::chrono::steady_clock::now() + std::chrono::minutes(1);
+
+  MallocTrimPeriod = Opts.MemoryCleanupPeriod;
+  NextMallocTrim = std::chrono::steady_clock::now() + MallocTrimPeriod;
 }
 
 ClangdLSPServer::~ClangdLSPServer() {
@@ -1615,6 +1638,10 @@
 void ClangdLSPServer::onBackgroundIndexProgress(
     const BackgroundQueue::Stats &Stats) {
   static const char ProgressToken[] = "backgroundIndexProgress";
+
+  // The background index did some work, maybe we need to cleanup
+  maybeMallocTrim();
+
   std::lock_guard<std::mutex> Lock(BackgroundIndexProgressMutex);
 
   auto NotifyProgress = [this](const BackgroundQueue::Stats &Stats) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to