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

[Eric Christopher]

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).
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150526/d728810d/attachment.html>