nsStandardURL::nsSegmentEncoder constructor is slow

RESOLVED WORKSFORME

Status

()

P2
normal
RESOLVED WORKSFORME
17 years ago
3 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({perf})

Trunk
Future
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The constructor of nsStandardURL::nsSegmentEncoder took about 0.5% of the
non-idle main-thread page load time in a profile I did on a live list of URLs
similar to jrgm's tests.  Most of the callers were for the construction of URLs
for image loads.  Most of the time spent is within the charset converter manager
code.  I think we should either make the ccm a lot faster or make nsStandardURL
cache encoders (although one might think that would be the point of the ccm).
Created attachment 83939 [details]
jprof profile of time spent within nsSegmentEncoder constructor

Updated

17 years ago
Keywords: perf

Comment 2

17 years ago
i imagine optimizing the ccm would benefit other consumers as well, so that gets
my vote.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.1beta
Most of the calls to the ccm come from this one call site.  I'd love to make
lots of changes to the ccm (for performance and for code size/less xpcom), but
the real problem here is that we shouldn't be re-fetching the decoder for every
image uri in the document.  Could there be an nsIOService::newURI variant that
takes an encoder rather than a charset string?

Comment 4

17 years ago
no, that's not really an option for a couple of reasons:

1) nsIOService::newURI maps to nsIProtocolHandler::newURI and nsIProtocolHandler
is frozen.

2) we need to be able to provide the charset via nsIURI::originCharset

ok, so how about we put together our own little cache of charset converters.  we
could just keep around a simple linked list of the last 5 charset converters
used.  that would be fairly trivial to implement and should be enough of an
optimization.  we also don't need to worry about threadsafety here :)

Comment 5

17 years ago
dbaron: if we only wanted to optimize this codepath for ASCII URLs, we could 
simply delay calling into the ccm until mEncoder is really needed.  we are
already scanning the URL text to determine if calling into mEncoder is even
necessary, so it would be trivial to delay initialization of mEncoder.
Target Milestone: mozilla1.1beta → Future
From looking at the code all the CCM stuff is long gone, so this bug is likely moot.  Suggest it be closed as WFM
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.