[LLVMdev] RFC: Reduce the memory footprint of DIEs (and DIEValues)

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed Jun 24 13:11:39 PDT 2015


> On 2015 Jun 17, at 11:36, David Blaikie <dblaikie at gmail.com> wrote:
> 
>> On Wed, Jun 17, 2015 at 11:26 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> > On 2015 Jun 16, at 21:40, Yaron Keren <yaron.keren at gmail.com> wrote:
>> >
>> > Hi Duncan,
>> >
>> > Singly linked lists are used throught LLVM: ArrayRecycler, Registry, LiveInterval, SCEVUnknown, CGBlockInfo, MacroArgCache, ...DIEValueList.
>> >
>> > What do you think about implementing a singly-linked ADT?
>> >
>> 
>> Interesting idea.
>> 
>> I've taken a look at the other singly-linked lists you mentioned.  Most
>> of these are some version of a free/recycling list (ArrayRecycler,
>> Registry, MacroArgCache, etc.), which need super simple push/pop API and
>> no memory management.
>> 
>> LiveInterval and CGBlock also need iteration and a way to conditionally
>> remove elements (something like `remove_if()`), and in a few cases it
>> would be convenient to have API to destruct and/or destroy nodes.
>> 
>> I think these would all be straightforward to formalize into an ADT.  We
>> could even use it to implement a value-based `llvm::forward_list<>` that
>> avoided the sorting bug in libstdc++ (which gets "stable" exactly
>> backwards) that David hit when refactoring tablegen.
>> 
>> Unfortunately, I think it'd be hard to reuse for DIE::Children and
>> DIEValueList.  The latter wants to preserve insertion order and needs
>> `push_back()` API (the former can technically get away with a timely
>> call to `reverse()`, but it's awkward).  Supporting `push_back()`
>> efficiently requires pointing at the last element somehow, either via an
>> extra pointer or circular logic (what my latest patch does for
>> DIE::Children).
> 
> One of the other problems I hit when trying to value-ify the DIE children is that the point of DIE creation is, in some particular cases, divorced from the knowledge of which container it's going to go in to. Yet pointer identity is needed before then. 
> 
> At least that's how I recall it - may be worth going down that path again just so we can write it up clearly for posterity, I dunno.

Nope, I saw this too.

In theory, you could get std::list to work here, by inserting into a
singleton linked list when you first need the reference, and splicing
it into its parent's linked list once it's created.  However, I think a
value-based list is too expensive anyway (see below)...

> If it is possible, then I would love/prefer to use an existing non-intrusive container (or writing one if we have to). at least could prototype with std::list, then, as you say, implement a std::forward_list-like device with the bonus that we could fix sort and do the circular trick to allow insertion at the end, etc.

Unfortunately the BumpPtrAllocator/skip-unlinking-the-list part of these
patches is really necessary, making std::list pretty awkward.  My payload
usually takes ~2 minutes when profiling, and when I profiled linked lists
without the BumpPtrAllocator it was talking >15 minutes to teardown (I
killed it, so I don't know how much longer it would have taken).

Moreover, using a BumpPtrAllocator this way fixes the slow DIE teardown
that Fred was battling in llvm-dsymutil.

> (& sorry, will get to proper review of this patch soon... )

(Sadly, I completely missed this reply of yours until this morning (when
I was already preparing the patches below).)

I've rebased the patches on top of r240566, and squashed them together
with shared implementation for the two linked lists in die-diet.patch.

Yesterday (on top of 240244), I reran llc to check memory usage on the
verify-uselistorder.lto.opt.bc payload.  This patch drops peak memory
usage from 800MB down to 720MB, 10%.  I also timed before/after the patch
(no heap profile, CPU speed locked down, best of 5 runs) and measured a
1% speedup.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: die-diet.patch
Type: application/octet-stream
Size: 85112 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150624/592989ef/attachment.obj>
-------------- next part --------------


(I've also attached die-block-is-not-a-die.patch, which fixes the
DIELoc/DIEBlock/DIE relationship.  I've been holding off on committing
it since it's built on top of the die-diet.patch.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: die-block-is-not-a-die.patch
Type: application/octet-stream
Size: 11392 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150624/592989ef/attachment-0001.obj>
-------------- next part --------------

)

> 
>> We could configure this stuff via templates -- I'd be open to the idea
>> -- but I think the intersection between the implementations would be
>> practically nil.  Even the iterator implementations need to be
>> completely different.  If these are the only `push_back()`-enabled
>> slists in tree, is it premature to abstract it?  Would we even want the
>> same name as "normal" slists?
>> 
>> (Regardless, I think it would be a great idea for someone to ADT-ify the
>> other linked lists!)
> 


More information about the llvm-commits mailing list