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

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Darin Fisher]

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.

Comment 1

[Darin Fisher]

Attachment: v1 patch

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.

Comment 2

[Darin Fisher]

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

Comment 3

[Darin Fisher]

Comment on attachment 148401 [details] [diff] [review]
v1 patch

can i milk you for an r+sr=  boris?

Comment 4

[Darin Fisher]

This depends on my patch for bug 237819, which adds the IsACE call to
nsStandardURL ;-)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 148401 [details] [diff] [review]
v1 patch

Yeah, that seems reasonable.  r+sr=bzbarsky

Comment 6

[Darin Fisher]

fixed-on-trunk