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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to