Closed
Bug 265534
Opened 19 years ago
Closed 15 years ago
reduce inlining of string constructors
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: bzbarsky)
References
Details
(Keywords: memory-footprint)
Attachments
(1 file)
6.24 KB,
patch
|
benjamin
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
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.)
Comment 1•19 years ago
|
||
duplicate of bug 235104 ... want to take that one instead?
![]() |
Assignee | |
Comment 2•17 years ago
|
||
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.
Updated•17 years ago
|
QA Contact: string
![]() |
Assignee | |
Comment 3•15 years ago
|
||
Do we want to give this a shot on the perf tests?
Attachment #332363 -
Flags: superreview?(dbaron)
Attachment #332363 -
Flags: review?(benjamin)
Reporter | ||
Comment 4•15 years ago
|
||
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 6•15 years ago
|
||
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+
![]() |
Assignee | |
Comment 7•15 years ago
|
||
| 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.
Reporter | ||
Comment 8•15 years ago
|
||
Comment on attachment 332363 [details] [diff] [review] Patch doing what comment 3 describes sr=dbaron
Attachment #332363 -
Flags: superreview?(dbaron) → superreview+
![]() |
Assignee | |
Comment 9•15 years ago
|
||
Pushed as changeset 70c7932205ee.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•15 years ago
|
Assignee: dbaron → bzbarsky
Updated•2 years ago
|
Component: String → XPCOM
You need to log in
before you can comment on or make changes to this bug.
Description
•