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

RESOLVED FIXED in mozilla27

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla27
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 820361 [details] [diff] [review]
use template typedefs, not inheritance, to define nsTArray element copiers

2% codesize win on ARM Android is nothing to sneeze at.
Attachment #820361 - Flags: review?(ehsan)
(Assignee)

Comment 2

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

Comment 3

5 years ago
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.  :(
Attachment #820361 - Flags: review?(ehsan) → review+
(Assignee)

Comment 4

5 years ago
(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

5 years ago
(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++!
(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
https://hg.mozilla.org/mozilla-central/rev/8b72c1a37bd6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
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.
(Assignee)

Comment 10

5 years ago
(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

5 years ago
Does that mean that we don't use ICF on b2g?! o_O
(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.
Assignee: nobody → nfroyd
You need to log in before you can comment on or make changes to this bug.