Closed Bug 243498 Opened 20 years ago Closed 20 years ago

nsStandardURL::BuildNormalizedSpec spends too much time in nsIDNService::IsACE

Categories

(Core :: Networking, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.8alpha1

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

(Keywords: perf)

Attachments

(1 file)

nsStandardURL::BuildNormalizedSpec spends too much time in nsIDNService::IsACE.

I built a test program that calls SetSpec on a list of URLs scrapped from a web
browsing session in a loop.

The resulting jprof data was pretty interesting and consistent with the data
boris posted in bug 243234.

It turns out that a considerable part of the time spent is inside
nsIDNService::IsACE.  A quick inspection of that code shows that it uses some
pretty bad string fu.
Attached patch v1 patchSplinter Review
here's a patch that makes the cost associated with IsACE almost negligible.

hits on BuildNormalizedSpec in my profile went from 458 to 345 with this patch.
Here's the jprof output for IsACE w/o this patch:

 21493   8      150 nsIDNService::IsACE(nsACString const&, int*)
                 58 CaseInsensitiveFindInReadable(nsACString const&,
nsReadingIterator&, nsReadingIterator&)
                 55 nsACString::Assign(nsCSubstringTuple const&)
                 12 nsACString::~nsACString()
                 10 StringBeginsWith(nsACString const&, nsACString const&,
nsCStringComparator const&)

Most of the cost is associated with CaseInsensitiveFindInReadable and
constructing |NS_LITERAL_CSTRING(".") + prefix|.  My patch uses PL_strncasestr,
and results in this jprof output for IsACE:

 21494   2       28 nsIDNService::IsACE(nsACString const&, int*)
                 26 PL_strncasestr
Summary: nsStandardURL::BuildNormalizedSpec spends too much time in nsIDNService::IsACE → nsStandardURL::BuildNormalizedSpec spends too much time in nsIDNService::IsACE
Comment on attachment 148401 [details] [diff] [review]
v1 patch

can i milk you for an r+sr=  boris?
Attachment #148401 - Flags: superreview?(bzbarsky)
Attachment #148401 - Flags: review?(bzbarsky)
This depends on my patch for bug 237819, which adds the IsACE call to
nsStandardURL ;-)
Severity: normal → minor
Status: NEW → ASSIGNED
Depends on: 237819
Keywords: perf
Target Milestone: --- → mozilla1.8alpha
Comment on attachment 148401 [details] [diff] [review]
v1 patch

Yeah, that seems reasonable.  r+sr=bzbarsky
Attachment #148401 - Flags: superreview?(bzbarsky)
Attachment #148401 - Flags: superreview+
Attachment #148401 - Flags: review?(bzbarsky)
Attachment #148401 - Flags: review+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: