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

Tue May 26 15:30:00 PDT 2015

bcc:llvmdev, +llvm-commits

Thanks Eric!

David, let me know if you have any comments on the `DIE::Children`
stuff (patch 0008 in particular, although I think Eric wanted your
opinion on 0007 as well).

Fred, you mentioned in person you want to have a look.  I'll hold
off on committing until you have a chance too.

I've rebased the patches, removed some FIXMEs that I forgot to
delete, and added an 'all.patch' in case that's more convenient.

(I haven't addressed Eric's comments yet; I'm mainly reposting to
move this off of llvmdev.)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-AsmPrinter-Reorganize-DIE.h-NFC.patch
Type: application/octet-stream
Size: 9324 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-AsmPrinter-Change-DIEValue-to-be-stored-by-value.patch
Type: application/octet-stream
Size: 105293 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-AsmPrinter-Store-abbreviation-data-directly-in-DIE-a.patch
Type: application/octet-stream
Size: 23713 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-AsmPrinter-Stop-exposing-underlying-DIEValue-list-NF.patch
Type: application/octet-stream
Size: 13310 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-AsmPrinter-Return-added-DIE-from-DIE-addChild.patch
Type: application/octet-stream
Size: 1754 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-AsmPrinter-Stop-exposing-underlying-DIE-children-lis.patch
Type: application/octet-stream
Size: 4045 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-AsmPrinter-Convert-DIE-Values-to-a-linked-list.patch
Type: application/octet-stream
Size: 63510 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0006.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-AsmPrinter-Use-an-intrusively-linked-list-for-DIE-Ch.patch
Type: application/octet-stream
Size: 44101 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0007.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: all.patch
Type: application/octet-stream
Size: 153422 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/44857a1c/attachment-0008.obj>
-------------- next part --------------

> On 2015-May-26, at 14:41, Eric Christopher <echristo at gmail.com> wrote:
> 
> Hi Duncan,
> 
> Few random comments on things, in general looks pretty good.
> 
> 0001 - LGTM.
> 0002 - LGTM.
> 0003 -
> 
>    struct AttrEntry {
>      DIEValue Val;
> -    const DIEAbbrevData *Desc;
>    };
> 
> Most boring struct ever?
> 
> +  const DIEAbbrev &Abbrev = getAbbrev(Die.getAbbrevNumber());
> 
> This is a bit of an awkward construction. I understand where it's coming from, but can you go back and comment the various abbreviation bits with how it works now?
> 
> -  AssignAbbrev(Die->getAbbrev());
> +  AssignAbbrev(NewAbbrev);
> +  Die->setAbbrevNumber(NewAbbrev.getNumber());
> 
> To elaborate - the need for this sort of thing should be documented.
> 
> 0004 - LGTM.
> 0005 - Better commit message saying why you're going to want this more later?
> 0006 - LGTM.
> 0007 - LGTM, might want to have Dave take a look at it as well.
> 0008 - reverseChildren and finalizeChildren is pretty gross. Also, I'm going to punt on the rest of this to Dave.
> 
> Thanks for all the work!
> 
> -eric
> 
> On Sun, May 24, 2015 at 11:55 AM Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> > On 2015 May 20, at 17:51, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> >
> >
> >> On 2015 May 20, at 17:39, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> >>
> >> With just those four patches, memory usage went *up* slightly.  Add in
> >> the 5th patch (which does #2 below), and we get an overall memory drop
> >> of 4%.
> >
> > Forgot to post numbers for this.  Peak memory was at 920 MB before
> > the five patches, and 884 MB after.  (These exact numbers won't quite
> > reproduce in ToT since I still haven't finished cleaning up and
> > committing the MCSymbol and emitLabelDiff() work I hacked on top of,
> > but the 36 MB drop should hold.)
> 
> I've cleaned all this up and committed the most obvious parts, as well
> as a few unrelated memory improvements.  I'm attaching my (almost?)
> ready-to-go patches, which have the following effects on peak memory:
> 
>   - 0000: 845 MB (baseline)
>   - 0001: 845 MB - refactor
>   - 0002: 879 MB - pass DIEValue by value (momentary setback)
>   - 0003: 829 MB - merge DIEAbbrevData into DIEValue
>   - 0004: 829 MB - refactor
>   - 0005: 829 MB - refactor
>   - 0006: 829 MB - refactor
>   - 0007: 764 MB - change DIE::Values to a linked list
>   - 0008: 756 MB - change DIE::Children to a linked list
> 
> (Still measuring memory on `llc` for `-flto -g`; details in r236629.)
> 
> @Eric, you mentioned offline you'd like to have a look at this proposal
> before I proceed -- obviously I've been impatient ;).  Let me know if
> I'm okay to move forward and start committing (modulo a couple of these
> that I'll want a review from David and Fred on).
> 