Merged main:75a3f326c3d8 into amd-gfx:e5e111cf62ac

Local branch amd-gfx e5e111c Merged main:7ec7788ac175 into amd-gfx:52b101124f00
Remote branch main 75a3f32 [IR] Add an ImplicitLocOpBuilder helper class for building IR with the same loc.

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -1285,13 +1285,7 @@ void ClangdLSPServer::publishDiagnostics(
 }
 
 void ClangdLSPServer::maybeExportMemoryProfile() {
-  if (!trace::enabled())
-    return;
-  // Profiling might be expensive, so we throttle it to happen once every 5
-  // minutes.
-  static constexpr auto ProfileInterval = std::chrono::minutes(5);
-  auto Now = std::chrono::steady_clock::now();
-  if (Now < NextProfileTime)
+  if (!trace::enabled() || !ShouldProfile())
     return;
 
   static constexpr trace::Metric MemoryUsage(
@@ -1300,27 +1294,11 @@ void ClangdLSPServer::maybeExportMemoryProfile() {
   MemoryTree MT;
   profile(MT);
   record(MT, "clangd_lsp_server", MemoryUsage);
-  NextProfileTime = Now + ProfileInterval;
 }
 
 void ClangdLSPServer::maybeCleanupMemory() {
-  // Memory cleanup is probably expensive, throttle it
-  static constexpr auto MemoryCleanupInterval = std::chrono::minutes(1);
-
-  if (!Opts.MemoryCleanup)
+  if (!Opts.MemoryCleanup || !ShouldCleanupMemory())
     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();
 }
 
@@ -1481,10 +1459,15 @@ void ClangdLSPServer::onAST(const ASTParams &Params,
 ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
                                  const ThreadsafeFS &TFS,
                                  const ClangdLSPServer::Options &Opts)
-    : BackgroundContext(Context::current().clone()), Transp(Transp),
+    : ShouldProfile(/*Period=*/std::chrono::minutes(5),
+                    /*Delay=*/std::chrono::minutes(1)),
+      ShouldCleanupMemory(/*Period=*/std::chrono::minutes(1),
+                          /*Delay=*/std::chrono::minutes(1)),
+      BackgroundContext(Context::current().clone()), Transp(Transp),
       MsgHandler(new MessageHandler(*this)), TFS(TFS),
       SupportedSymbolKinds(defaultSymbolKinds()),
       SupportedCompletionItemKinds(defaultCompletionItemKinds()), Opts(Opts) {
+
   // clang-format off
   MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
   MsgHandler->bind("initialized", &ClangdLSPServer::onInitialized);
@@ -1529,10 +1512,6 @@ ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
   if (Opts.FoldingRanges)
     MsgHandler->bind("textDocument/foldingRange", &ClangdLSPServer::onFoldingRange);
   // clang-format on
-
-  // 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() {

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -19,6 +19,7 @@
 #include "support/Context.h"
 #include "support/MemoryTree.h"
 #include "support/Path.h"
+#include "support/Threading.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSet.h"
@@ -186,18 +187,12 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
   /// Runs profiling and exports memory usage metrics if tracing is enabled and
   /// profiling hasn't happened recently.
   void maybeExportMemoryProfile();
+  PeriodicThrottler ShouldProfile;
 
   /// 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;
+  PeriodicThrottler ShouldCleanupMemory;
 
   /// Since initialization of CDBs and ClangdServer is done lazily, the
   /// following context captures the one used while creating ClangdLSPServer and

clang-tools-extra/clangd/support/Threading.cpp

@@ -116,5 +116,17 @@ void wait(std::unique_lock<std::mutex> &Lock, std::condition_variable &CV,
   CV.wait_until(Lock, D.time());
 }
 
+bool PeriodicThrottler::operator()() {
+  Rep Now = Stopwatch::now().time_since_epoch().count();
+  Rep OldNext = Next.load(std::memory_order_acquire);
+  if (Now < OldNext)
+    return false;
+  // We're ready to run (but may be racing other threads).
+  // Work out the updated target time, and run if we successfully bump it.
+  Rep NewNext = Now + Period;
+  return Next.compare_exchange_strong(OldNext, NewNext,
+                                      std::memory_order_acq_rel);
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/support/Threading.h

@@ -169,6 +169,35 @@ template <typename Container> class Memoize {
   }
 };
 
+/// Used to guard an operation that should run at most every N seconds.
+///
+/// Usage:
+///   mutable PeriodicThrottler ShouldLog(std::chrono::seconds(1));
+///   void calledFrequently() {
+///     if (ShouldLog())
+///       log("this is not spammy");
+///   }
+///
+/// This class is threadsafe. If multiple threads are involved, then the guarded
+/// operation still needs to be threadsafe!
+class PeriodicThrottler {
+  using Stopwatch = std::chrono::steady_clock;
+  using Rep = Stopwatch::duration::rep;
+
+  Rep Period;
+  std::atomic<Rep> Next;
+
+public:
+  /// If Period is zero, the throttler will return true every time.
+  PeriodicThrottler(Stopwatch::duration Period, Stopwatch::duration Delay = {})
+      : Period(Period.count()),
+        Next((Stopwatch::now() + Delay).time_since_epoch().count()) {}
+
+  /// Returns whether the operation should run at this time.
+  /// operator() is safe to call concurrently.
+  bool operator()();
+};
+
 } // namespace clangd
 } // namespace clang
 #endif

clang-tools-extra/clangd/unittests/support/ThreadingTests.cpp

@@ -10,6 +10,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <chrono>
 #include <mutex>
 
 namespace clang {
@@ -121,5 +122,25 @@ TEST_F(ThreadingTest, MemoizeDeterministic) {
   ASSERT_THAT(ValueA.load(), testing::AnyOf('A', 'B'));
 }
 
+// It's hard to write a real test of this class, std::chrono is awkward to mock.
+// But test some degenerate cases at least.
+TEST(PeriodicThrottlerTest, Minimal) {
+  PeriodicThrottler Once(std::chrono::hours(24));
+  EXPECT_TRUE(Once());
+  EXPECT_FALSE(Once());
+  EXPECT_FALSE(Once());
+
+  PeriodicThrottler Later(std::chrono::hours(24),
+                          /*Delay=*/std::chrono::hours(24));
+  EXPECT_FALSE(Later());
+  EXPECT_FALSE(Later());
+  EXPECT_FALSE(Later());
+
+  PeriodicThrottler Always(std::chrono::seconds(0));
+  EXPECT_TRUE(Always());
+  EXPECT_TRUE(Always());
+  EXPECT_TRUE(Always());
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

@@ -47,7 +47,17 @@ Major New Features
 Improvements to clangd
 ----------------------
 
-The improvements are...
+- clangd's memory usage is significantly reduced on most Linux systems.
+  In particular, memory usage should not increase dramatically over time.
+
+  The standard allocator on most systems is glibc's ptmalloc2, and it creates
+  disproportionately large heaps when handling clangd's allocation patterns.
+  By default, clangd will now periodically call ``malloc_trim`` to release free
+  pages on glibc systems.
+
+  Users of other allocators (such as ``jemalloc`` or ``tcmalloc``) on glibc
+  systems can disable this using ``--malloc_trim=0`` or the CMake flag
+  ``-DCLANGD_MALLOC_TRIM=0``.
 
 Improvements to clang-doc
 -------------------------

lld/MachO/SyntheticSections.cpp

@@ -694,6 +694,11 @@ void SymtabSection::emitStabs() {
 }
 
 void SymtabSection::finalizeContents() {
+  auto addSymbol = [&](std::vector<SymtabEntry> &symbols, Symbol *sym) {
+    uint32_t strx = stringTableSection.addString(sym->getName());
+    symbols.push_back({sym, strx});
+  };
+
   // Local symbols aren't in the SymbolTable, so we walk the list of object
   // files to gather them.
   for (InputFile *file : inputFiles) {
@@ -702,30 +707,25 @@ void SymtabSection::finalizeContents() {
         // TODO: when we implement -dead_strip, we should filter out symbols
         // that belong to dead sections.
         if (auto *defined = dyn_cast<Defined>(sym)) {
-          if (!defined->isExternal()) {
-            uint32_t strx = stringTableSection.addString(sym->getName());
-            localSymbols.push_back({sym, strx});
-          }
+          if (!defined->isExternal())
+            addSymbol(localSymbols, sym);
         }
       }
     }
   }
 
   // __dyld_private is a local symbol too. It's linker-created and doesn't
   // exist in any object file.
-  if (Defined* dyldPrivate = in.stubHelper->dyldPrivate) {
-    uint32_t strx = stringTableSection.addString(dyldPrivate->getName());
-    localSymbols.push_back({dyldPrivate, strx});
-  }
+  if (Defined* dyldPrivate = in.stubHelper->dyldPrivate)
+    addSymbol(localSymbols, dyldPrivate);
 
   for (Symbol *sym : symtab->getSymbols()) {
-    uint32_t strx = stringTableSection.addString(sym->getName());
     if (auto *defined = dyn_cast<Defined>(sym)) {
       assert(defined->isExternal());
-      externalSymbols.push_back({sym, strx});
+      addSymbol(externalSymbols, sym);
     } else if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
       if (dysym->isReferenced())
-        undefinedSymbols.push_back({sym, strx});
+        addSymbol(undefinedSymbols, sym);
     }
   }
 

lld/test/MachO/symtab.s

@@ -86,17 +86,19 @@
 # CHECK-NEXT:   iundefsym: 5
 # CHECK-NEXT:   nundefsym: 2
 
-## Verify that the first entry in the StringTable is a space.
+## Verify that the first entry in the StringTable is a space, and that
+## unreferenced symbols aren't emitted.
 # RUN: obj2yaml %t/test | FileCheck %s --check-prefix=YAML
 # YAML:      StringTable:
 # YAML-NEXT: ' '
+# YAML-NOT: _unreferenced
 
 #--- libfoo.s
 .globl _dynamic
 _dynamic:
 
 #--- test.s
-.globl _main, _external, _external_weak
+.globl _main, _external, _external_weak, _unreferenced
 
 .data
 _external:

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h

@@ -194,7 +194,7 @@ inline unsigned GetDefaultP2AlignAny(unsigned Opc) {
   WASM_LOAD_STORE(ATOMIC_RMW8_U_XCHG_I64)
   WASM_LOAD_STORE(ATOMIC_RMW8_U_CMPXCHG_I32)
   WASM_LOAD_STORE(ATOMIC_RMW8_U_CMPXCHG_I64)
-  WASM_LOAD_STORE(LOAD_SPLAT_v8x16)
+  WASM_LOAD_STORE(LOAD8_SPLAT)
   WASM_LOAD_STORE(LOAD_LANE_v16i8)
   WASM_LOAD_STORE(STORE_LANE_v16i8)
   return 0;
@@ -222,7 +222,7 @@ inline unsigned GetDefaultP2AlignAny(unsigned Opc) {
   WASM_LOAD_STORE(ATOMIC_RMW16_U_XCHG_I64)
   WASM_LOAD_STORE(ATOMIC_RMW16_U_CMPXCHG_I32)
   WASM_LOAD_STORE(ATOMIC_RMW16_U_CMPXCHG_I64)
-  WASM_LOAD_STORE(LOAD_SPLAT_v16x8)
+  WASM_LOAD_STORE(LOAD16_SPLAT)
   WASM_LOAD_STORE(LOAD_LANE_v8i16)
   WASM_LOAD_STORE(STORE_LANE_v8i16)
   return 1;
@@ -253,7 +253,7 @@ inline unsigned GetDefaultP2AlignAny(unsigned Opc) {
   WASM_LOAD_STORE(ATOMIC_RMW32_U_CMPXCHG_I64)
   WASM_LOAD_STORE(MEMORY_ATOMIC_NOTIFY)
   WASM_LOAD_STORE(MEMORY_ATOMIC_WAIT32)
-  WASM_LOAD_STORE(LOAD_SPLAT_v32x4)
+  WASM_LOAD_STORE(LOAD32_SPLAT)
   WASM_LOAD_STORE(LOAD_ZERO_v4i32)
   WASM_LOAD_STORE(LOAD_LANE_v4i32)
   WASM_LOAD_STORE(STORE_LANE_v4i32)
@@ -272,7 +272,7 @@ inline unsigned GetDefaultP2AlignAny(unsigned Opc) {
   WASM_LOAD_STORE(ATOMIC_RMW_XCHG_I64)
   WASM_LOAD_STORE(ATOMIC_RMW_CMPXCHG_I64)
   WASM_LOAD_STORE(MEMORY_ATOMIC_WAIT64)
-  WASM_LOAD_STORE(LOAD_SPLAT_v64x2)
+  WASM_LOAD_STORE(LOAD64_SPLAT)
   WASM_LOAD_STORE(LOAD_EXTEND_S_v8i16)
   WASM_LOAD_STORE(LOAD_EXTEND_U_v8i16)
   WASM_LOAD_STORE(LOAD_EXTEND_S_v4i32)