nsHttpHandler uses nsXPIDLCString for no apparent reason

VERIFIED FIXED in Future

Status

()

Core
Networking: HTTP
--
trivial
VERIFIED FIXED
16 years ago
13 years ago

People

(Reporter: Biesinger, Assigned: Mikael Parknert)

Tracking

({perf})

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.19 KB, patch
Biesinger
: review+
Darin Fisher
: superreview+
Details | Diff | Splinter Review
Almost all string members of nsHttpHandler are nsXPIDLCStrings, even though at
least some of them are never used in IDL calls; they are only assigned to.
Also, these strings are assigned to using .Adopt(nsCRT::strdup(...));

This could be made much faster if nsCAutoString and .Assign would be used.

This is true for at least these two members:
mPlatform mOscpu
Other members which can be changed:
mAppname
mAppVersion
mPlatform
mLanguage

And the others should probably use .Assign too, instead of .Adopt

Comment 2

16 years ago
the reason stems from the fact that many of the UA components are set via
preferences, and we use getter_Copies to get string values from prefs.

of course, not all string values are set via prefs as you've noticed.  using
nsCAutoString here is wasteful since the values are member variables, and
allocation of the UA components is only done really once during startup.

this is hardly a significant issue, but it probably should be cleaned up.
Severity: normal → trivial
Status: NEW → ASSIGNED
Target Milestone: --- → Future
(Assignee)

Updated

13 years ago
Assignee: darin → mikael
Status: ASSIGNED → NEW
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

13 years ago
OS: Linux → All
Hardware: PC → All
(Assignee)

Comment 3

13 years ago
Created attachment 172011 [details] [diff] [review]
patch
Attachment #172011 - Flags: superreview?(darin)
Attachment #172011 - Flags: review?(cbiesinger)
Comment on attachment 172011 [details] [diff] [review]
patch

hm... can you make the variable names align with the other ones? compare
mUserAgent.

r=biesi with that change; please attach a new patch with it
Attachment #172011 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 5

13 years ago
Created attachment 172039 [details] [diff] [review]
updated with indent
Comment on attachment 172039 [details] [diff] [review]
updated with indent

thanks
Attachment #172039 - Flags: review+
(Assignee)

Updated

13 years ago
Attachment #172011 - Attachment is obsolete: true
Attachment #172011 - Flags: superreview?(darin)
(Assignee)

Updated

13 years ago
Attachment #172039 - Flags: superreview?(darin)

Comment 7

13 years ago
Comment on attachment 172039 [details] [diff] [review]
updated with indent

sr=darin
Attachment #172039 - Flags: superreview?(darin) → superreview+
I checked this patch in:
Checking in netwerk/protocol/http/src/nsHttpHandler.h;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpHandler.h,v  <--  nsHttpHandler.h
new revision: 1.44; previous revision: 1.43
done
(Assignee)

Updated

13 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Keywords: perf

Comment 9

13 years ago
V. lxr.
Status: RESOLVED → VERIFIED
QA Contact: tever → benc
You need to log in before you can comment on or make changes to this bug.