188828 - nsAutoString's constructors are slow

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch, ugly version

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.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Except that wouldn't handle the CBufDescriptor case very easily...

Comment 3

[Alec Flett]

feel free to rip apart nsStrPrivate::Inititalize - nobody likes
nsStr/nsStrPrivate  anyway.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(So is this patch too ugly to land?)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Previous patch, updated to trunk.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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...

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 139855 [details] [diff] [review]
patch

Actually, never mind.  I can get rid of the null checks.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 10

[Alec Flett]

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

[Darin Fisher]

Comment on attachment 139858 [details] [diff] [review]
patch

r=darin, though i'm hoping to change all this code shortly ;)

Comment 12

[Alec Flett]

Comment on attachment 139858 [details] [diff] [review]
patch

sr=alecf

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2004-01-28 20:00 -0800.