thr3ads.net - llvm dev - [LLVMdev] RFC: Reduce the memory footprint of DIEs (and DIEValues) [May 2015]

If this information is useful, please help other people find it:
Share via:

Duncan P. N. Exon Smith

2015-May-24 18:55 UTC

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

> 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 --------------
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-dev/attachments/20150524/bf7f761c/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: 105558 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150524/bf7f761c/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-dev/attachments/20150524/bf7f761c/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-dev/attachments/20150524/bf7f761c/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-dev/attachments/20150524/bf7f761c/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-dev/attachments/20150524/bf7f761c/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: 63526 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150524/bf7f761c/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-dev/attachments/20150524/bf7f761c/attachment-0007.obj>

Yaron Keren

2015-May-25 06:03 UTC

head link

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

>   - 0000: 845 MB (baseline)
>   - 0008: 756 MB - change DIE::Children to a linked list
That's really nice savings, more than 10% !


2015-05-24 21:55 GMT+03:00 Duncan P. N. Exon Smith <dexonsmith at
apple.com>:>
>
> > 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).
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150525/a39cd502/attachment.html>

Eric Christopher

2015-May-26 21:41 UTC

head link

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

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>

Duncan P. N. Exon Smith

2015-May-26 22:30 UTC

head link

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

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-dev/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-dev/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-dev/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-dev/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-dev/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-dev/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-dev/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-dev/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-dev/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).
>

llvm dev - May 2015 - [LLVMdev] RFC: Reduce the memory footprint of DIEs (and DIEValues)

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

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

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

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