Closed
Bug 188828
Opened 22 years ago
Closed 21 years ago
nsAutoString's constructors are slow
Categories
(Core :: XPCOM, defect, P2)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla1.7alpha
People
(Reporter: dbaron, Assigned: dbaron)
Details
(Keywords: perf, Whiteboard: [patch])
Attachments
(1 file, 2 obsolete files)
12.82 KB,
patch
|
darin.moz
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
nsAutoString's constructors have always shown up in profiles more than one would think they ought to. I'll attach a patch I wrote to attempt to speed them up, written while investigating bug 188224, using an idea I described in bug 162017 comment 20.
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [patch]
Target Milestone: --- → mozilla1.3beta
Assignee | ||
Comment 1•22 years ago
|
||
This is the ugly version. I'll make a cleaner version later -- what I should really do is just make an inline, protected, constructor of ns[C]String that takes the same parameters as nsStrPrivate::Initialize and calls it with those parameters.
Assignee | ||
Comment 2•22 years ago
|
||
Except that wouldn't handle the CBufDescriptor case very easily...
Comment 3•22 years ago
|
||
feel free to rip apart nsStrPrivate::Inititalize - nobody likes nsStr/nsStrPrivate anyway.
Assignee | ||
Comment 5•21 years ago
|
||
Previous patch, updated to trunk.
Attachment #111368 -
Attachment is obsolete: true
Assignee | ||
Comment 6•21 years ago
|
||
Comment on attachment 139855 [details] [diff] [review] patch This is ugly, but it should be a measurable performance win. Based on my memory from quite a while ago and comment 3, it would be hard to do this more cleanly without getting rid of CBufDescriptor, although I don't remember why...
Attachment #139855 -
Flags: review?(darin)
Assignee | ||
Comment 7•21 years ago
|
||
Ah, right. What makes it hard is that the constructor taking a CBufDescriptor does the initialization in one of two different ways, which is hard to do with constructor calls (although it is possible, I admit, but perhaps even uglier than this).
Assignee | ||
Comment 8•21 years ago
|
||
Comment on attachment 139855 [details] [diff] [review] patch Actually, never mind. I can get rid of the null checks.
Attachment #139855 -
Flags: review?(darin)
Assignee | ||
Updated•21 years ago
|
Attachment #139858 -
Flags: review?(darin)
Assignee | ||
Updated•21 years ago
|
Target Milestone: mozilla1.3beta → mozilla1.7alpha
Comment 10•21 years ago
|
||
heh, funny - I feel like I had a similar patch in my tree at one point maybe a year ago.. I like everything that is done here. oh, to be rid of eCharSize once and for all... *sigh* :)
Comment 11•21 years ago
|
||
Comment on attachment 139858 [details] [diff] [review] patch r=darin, though i'm hoping to change all this code shortly ;)
Attachment #139858 -
Flags: review?(darin) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #139858 -
Flags: superreview?(alecf)
Comment 12•21 years ago
|
||
Comment on attachment 139858 [details] [diff] [review] patch sr=alecf
Attachment #139858 -
Flags: superreview?(alecf) → superreview+
Assignee | ||
Comment 13•21 years ago
|
||
Fix checked in to trunk, 2004-01-28 20:00 -0800.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
Component: String → XPCOM
You need to log in
before you can comment on or make changes to this bug.
Description
•