929494 - use template typedefs, not inheritance, to define nsTArray element copiers

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

There's no reason to use inheritance here, and using plain typedefs avoids massive
amounts of code duplication for the common case of copying with mem*.  Code savings
on Android come in at about 570K (!), or ~2% of libxul .text size, which is a massive
win.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: use template typedefs, not inheritance, to define nsTArray element copiers

2% codesize win on ARM Android is nothing to sneeze at.

Comment 2

[Nathan Froyd [:froydnj]]

Benjamin and Taras might be interested in the codesize wins here.

Comment 3

[(no longer active)]

Comment on attachment 820361 [details] [diff] [review]
use template typedefs, not inheritance, to define nsTArray element copiers

Review of attachment 820361 [details] [diff] [review]:
-----------------------------------------------------------------

It is so very sad that this buys us any code size.  :(

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #3)
> It is so very sad that this buys us any code size.  :(

It oughtn't to on pgo win32; identical code folding should take care of that.  But for all our other platforms...

(Should be some compilation benefits, too, fewer templates/functions and all that.)

Comment 5

[(no longer active)]

(In reply to comment #4)
> (In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #3)
> > It is so very sad that this buys us any code size.  :(
> 
> It oughtn't to on pgo win32; identical code folding should take care of that. 
> But for all our other platforms...
> 
> (Should be some compilation benefits, too, fewer templates/functions and all
> that.)

Too many should's in the above statements...  Thanks, C++!

Comment 6

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b72c1a37bd6

Comment 7

[Brendan Eich [:brendan]]

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #5)
> Too many should's in the above statements...  Thanks, C++!

Without sarcasm: Thanks, Microsoft!

/be

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8b72c1a37bd6

Comment 9

[Mike Hommey [:glandium]]

It's surprising this makes a difference on android, as we're building with ICF, there. Did you measure that on a local build? Because we do now disable ICF on local builds, because it slows down linking.

Comment 10

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #9)
> It's surprising this makes a difference on android, as we're building with
> ICF, there. Did you measure that on a local build? Because we do now disable
> ICF on local builds, because it slows down linking.

Mmm, I did.  (Also didn't check the szip'd result.)  Well, B2G and Mac builds win libxul size, and everybody else gets smaller objects! ;)

Comment 11

[(no longer active)]

Does that mean that we don't use ICF on b2g?! o_O

Comment 12

[Mike Hommey [:glandium]]

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #11)
> Does that mean that we don't use ICF on b2g?! o_O

According to the b2g emulator opt build's configure log I just looked, we do use ICF on b2g.