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



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

Reply via email to