[Lldb-commits] [PATCH] D68549: make ConstString allocate memory in non-tiny chunks

Wed Oct 16 01:57:05 PDT 2019

llunak added a comment.

Oh, I somehow completely missed the fact that there are 256 of those :-/. In that case my numbers are way too much. Can you easily benchmark with different numbers? I think something like 131072 for BumpPtrAllocator (to still be large enough for the mmap threshold) could be reasonable, the size for StringPool can go down to 256 or maybe can be simply dropped.

In D68549#1710293 <https://reviews.llvm.org/D68549#1710293>, @joerg wrote:

> I'm a bit puzzled by the need for this change.

I could measure a difference in lldb symbol load times with the change (and it was enough to measure just with a stopwatch). And there's no "need" for the change, the logic was that it should improve situations that have many debug symbols and shouldn't matter for situations with few debug symbols. The assumption was that in realistic scenarios nobody would care that a debugging session needs 300KiB of memory instead of 100KiB. If I try to debug either a small test app or the whole clang binary, I can see a difference but nothing I'd care about in practice. What I care about though is that LLDB spends a significant portion of its startup time in ConstString.

In D68549#1710293 <https://reviews.llvm.org/D68549#1710293>, @joerg wrote:

> The bump allocator already has logic to do power-of-two scaling for the allocation, so I wonder why it doesn't work properly here.

As already said, it does work, eventually, but it's not power-of-two and so it takes its time before it figures out that it doesn't need to hammer on the allocation routines so much.

> Anyway, I think we either make the slab allocation logic smarter than just creating 1MiB slabs and trust the kernel/libc to not actually allocate that much real memory (maybe something like the std::vector growth model) or we fix the fact that we always have 256 string pools.

So, in what realistic scenarios is this change a problem? Maybe it'd be enough to just have #if defined(ANDROID) || defined(ARM) || defined(WHATEVER) and not overcomplicate this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549