[PATCH] D93452: [clangd] Trim memory periodically

[Sam McCall via Phabricator via llvm-commits]

sammccall added a comment.

In D93452#2460646 <https://reviews.llvm.org/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 investigated this, I found that replacing `DenseMap` and `StringMap` wtih `std::map` mitigates the issue. I think allocating huge chunks of contiguous memory temporarily make this issue worse. Not very useful, but hey, I'm just sharing what I observed. As I previously said, resolving this problem by changing containers or allocators is a lot of work and is not guaranteed to actually solve the issue.

@qchateau Hey, do you happen to remember *which* maps were changed? (No need to go through and repeat the investigation, just if you know).

I'm happy with how we resolved this robustly, but the fact that this is largely triggered by a few specific allocations is an unresolved mystery, and *maybe* there's room for more efficiency (if the malloc_trim is undoing work that need never be done).

My best guess is that hashtables in the index are both large (and thus likely allocated at the top of the arena rather than fitting into a free chunk) and long-lived (and thus block automatic arena shrinkage for a long time). And because the index tends to grow over time, it probably never fits into the old big gap!

In this case I guess there's nothing to do - the index snapshots need lots of memory, and allocating it in a huge chunk is probably optimal, even if we need to "hollow it out" with malloc_trim afterwards. A DenseMap that allows the hashtable to be split over multiple allocations might be interesting, but is obviously a massive project.
But curious if you have thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93452