Closed Bug 265534 Opened 17 years ago Closed 13 years ago
reduce inlining of string constructors
String class constructors, particularly those that take parameters, should be inlined much less. This could be a *huge* codesize improvement (and thus probably performance improvement). (While doing this, it might help to make nsAutoString128, nsAutoString256, etc., so that nsFixedString's constructors can be protected.)
duplicate of bug 235104 ... want to take that one instead?
Some data: Attachment 245232 [details] [diff] reduced codesize of gklayout by about 40KB. I tried taking all the other ctors of nsTSubstring_CharT our of line, and that only got me another 13KB in gklayout (compiling with -Os, stripped .so size). Then I tried taking the ctors of nsTFixedString_CharT out of line, and that got me another 58KB or so. This is all on a baseline size of about 5.3MB for gklayout.
Do we want to give this a shot on the perf tests?
One other thing worth experimenting with is seeing whether it's worth having constructors that are the base class constructors used by derived classes should be inline (and protected) so that we get a single constructor call rather than a whole bunch.
Comment on attachment 332363 [details] [diff] [review] Patch doing what comment 3 describes Give it a go on the tryserver for perf numbers?
Attachment #332363 - Flags: review?(benjamin) → review+
| mozilla-central | with patch ---------------------------------------------------- Linux Ts | 1556.95 | 1570.74 Linux Tp | 452.06 | 448.92 Mac Ts | 1238.79 or 1240.58 | 1232.47 or 1327.84 Mac Tp | 313.32 or 314.50 | 313.11 or 315.75 The Windows tryserver decided not to build my baseline for some reason (though it built things that came both before and after it on the other tryservers), but the with-patch numbers were: Ts: 1397.32 Tp: 442.30 These are not statistically different from the numbers for the other patches being tried. So it looks to me like this is a wash performance-wise.
Comment on attachment 332363 [details] [diff] [review] Patch doing what comment 3 describes sr=dbaron
Attachment #332363 - Flags: superreview?(dbaron) → superreview+
Pushed as changeset 70c7932205ee.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.