[PATCH] D93452: [clangd] Trim memory periodically

qchateau added a comment. If you look back at my original comment, you'll notice I originally trimmed the memory after `FileSymbols::buildIndex` which seemed to be the "end of the peak" memory usage, the goal was to counteract exactly what you describe: a huge memory usage when clangd warms-up

[PATCH] D93452: [clangd] Trim memory periodically

njames93 added a comment. A little follow up, but I've noticed if I have a lot of tabs open in my editor (usually happens as I forget to close them). When clangd starts (or restarts) , in the first minute, memory usage will shoot right up as it starts to do its thing. I've regularly seen it go o

[PATCH] D93452: [clangd] Trim memory periodically

sammccall added a comment. I tried to sketch a couple of tetris-based mental models for why these hashtables might never fit inside an existing gap, leading us to stack our arena size (tetris field height) higher and higher. This is predicated on the assumption that the index hashtables are the

[PATCH] D93452: [clangd] Trim memory periodically

sammccall added a comment. In D93452#2460646 , @qchateau wrote: > It does indeed look like facing the problem from the wrong side (fight the > end result instead of finding the root cause) but at that point it does solve > an annoying problem. As I inves

[PATCH] D93452: [clangd] Trim memory periodically

sammccall added a comment. In D93452#2467471 , @njames93 wrote: > I feel like we need some kind of entry for this in release notes given how > much of an issue it is. Good point. I usually procrastinate this and do a sweep of `git log` at release time,

[PATCH] D93452: [clangd] Trim memory periodically

njames93 added a comment. I feel like we need some kind of entry for this in release notes given how much of an issue it is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93452/new/ https://reviews.llvm.org/D93452

[PATCH] D93452: [clangd] Trim memory periodically

This revision was automatically updated to reflect the committed changes. Closed by commit rGb8c37153d539: [clangd] Trim memory periodically when using glibc malloc (authored by qchateau, committed by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D93452: [clangd] Trim memory periodically

sammccall accepted this revision. sammccall added a comment. In D93452#2467046 , @qchateau wrote: > I did not use the CMke option as the default value for the command line > option: IMO this CMake option is only useful if you encounter build problems > r

[PATCH] D93452: [clangd] Trim memory periodically

qchateau updated this revision to Diff 313215. qchateau marked 3 inline comments as done. qchateau added a comment. The log is back, I renamed the CMake option and the command line option and reduced the number of preprocessor directives. I chose not to modify the files for the `gn` build system,

[PATCH] D93452: [clangd] Trim memory periodically

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Awesome! Some nits on naming and stuff, but the only real substantial question for me is the exact availability/behavior of the command-line flag. Even there, as long as the default beha

[PATCH] D93452: [clangd] Trim memory periodically

qchateau added a comment. Here is the latest version, I believe it should mostly look like what you expect. Let me know if there is anything that you'd like me to change, it's totally fine to have a few more iterations on this diff. I added a `CLANGD_ENABLE_MEMORY_CLEANUP` to make sure people c

[PATCH] D93452: [clangd] Trim memory periodically

qchateau updated this revision to Diff 313000. qchateau marked 16 inline comments as done. qchateau added a comment. - Move platform specific code to ClangdMain - Generic memory cleanup callback in ClangdLSPServer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D93452: [clangd] Trim memory periodically

qchateau added a comment. > Sorry to mess you around like this, but I think I might have jumped the gun > in asking this to be abstracted away in `Process.h`. No problem, there is not much code anyway. The thought process behind the "right" solution is by far the most important. > So I think m

[PATCH] D93452: [clangd] Trim memory periodically

sammccall added a comment. Sorry to mess you around like this, but I think I might have jumped the gun in asking this to be abstracted away in `Process.h`. My reasoning is: - Once we are injecting the cleanup function from ClangdMain, that's not actually a crazy place to put this kind of system

[PATCH] D93452: [clangd] Trim memory periodically

qchateau added a comment. > Interestingly clangd held rock solid at 1.0GB recorded memory usage > throughout. What do you mean exactly ? Using the built in memory usage measurement ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93452/new/ https:

[PATCH] D93452: [clangd] Trim memory periodically

njames93 added a comment. Just attached a debugger and ran malloc_trim on my clangd instance which was sitting at 6.7GB, afterwards it was 1.3GB. Interestingly clangd held rock solid at 1.0GB recorded memory usage throughout. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D93452: [clangd] Trim memory periodically

qchateau planned changes to this revision. qchateau marked 2 inline comments as done. qchateau added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:52 +// Malloc trim minimal period +std::chrono::seconds MemoryCleanupPeriod = std::chrono::seconds(

[PATCH] D93452: [clangd] Trim memory periodically

sammccall added a comment. Thanks, this looks much closer to something we can land. Bunch of nits :-) Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1306 + + { +auto Now = std::chrono::steady_clock::now(); can you leave a FIXME to do this without

[PATCH] D93452: [clangd] Trim memory periodically

MaskRay added inline comments. Comment at: llvm/lib/Support/Unix/Process.inc:122 +#if defined(HAVE_MALLOC_TRIM) + return malloc_trim(Pad); +#else If the user uses jemalloc/tcmalloc with glibc, malloc_trim may exist while the function may do nothing. Repositor

[PATCH] D93452: [clangd] Trim memory periodically

qchateau updated this revision to Diff 312853. qchateau added a comment. - Fix clang-tidy errors 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