Closed
Bug 11677
Opened 26 years ago
Closed 25 years ago
nsStdURL::ReconstructSpec is slow -- too many nsString::Appends
Categories
(Core :: Networking, defect, P3)
Tracking
()
VERIFIED
FIXED
M12
People
(Reporter: sfraser_bugs, Assigned: gagan)
References
Details
(Whiteboard: [Perf])
Attachments
(1 file)
20.22 KB,
text/plain
|
Details |
Today's build is feeling very slow to me, rendering pages like Tinderbox, and even less stressing pages like Mozilla.org. On a fast Mac, I'm seeing a 4-5 second pause between setting the window title, and starting to display the page, on Tinderbox. The machine is unresponsive during this time. Dropping into the debugger during this delay shows the following stack: ... 02864590 PPC 15D7D06C nsParser::OnDataAvailable(nsIChannel*, nsISupports*, nsIInputStream*, unsigned int, unsigned int)+0027C 02864520 PPC 15D7C8F8 nsParser::ResumeParse(nsIDTD*, int)+0012C 028644E0 PPC 15D6D394 CNavDTD::WillInterruptParse()+00030 028644A0 PPC 152F0E30 HTMLContentSink::WillInterrupt()+000B4 02864450 PPC 152F9A70 nsHTMLDocument::ContentAppended(nsIContent*, int)+ 000EC 02864400 PPC 152B6E50 nsDocument::ContentAppended(nsIContent*, int)+00050 028643B0 PPC 152CFE84 PresShell::ContentAppended(nsIDocument*, nsIContent* , int)+00080 02864360 PPC 152E18D8 StyleSetImpl::ContentAppended(nsIPresContext*, nsIContent*, int)+00038 02864320 PPC 15558794 nsCSSFrameConstructor::ContentAppended(nsIPresContext*, nsIContent*, int)+004E8 028641C0 PPC 15557450 nsCSSFrameConstructor::ConstructFrame(nsIPresContext*, nsFrameConstructorState&, nsIContent*, nsIFrame*, int, nsFrameItems&)+00258 <snip recursion> 028630F0 PPC 15557450 nsCSSFrameConstructor::ConstructFrame(nsIPresContext*, nsFrameConstructorState&, nsIContent*, nsIFrame*, int, nsFrameItems&)+00258 02863050 PPC 15556A0C nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresContext*, nsFrameConstructorState&, const nsStyleDisplay*, nsIContent*, nsIFrame*, nsIStyleContext*, int, nsFrameItems&)+00A98 02862F10 PPC 1554CFE8 nsCSSFrameConstructor::ProcessChildren(nsIPresContext*, nsFrameConstructorState&, nsIContent*, nsIFrame*, int, nsFrameItems&)+00128 02862E90 PPC 155572C4 nsCSSFrameConstructor::ConstructFrame(nsIPresContext*, nsFrameConstructorState&, nsIContent*, nsIFrame*, int, nsFrameItems&)+000CC 02862DF0 PPC 1555712C nsCSSFrameConstructor::ResolveStyleContext(nsIPresContext*, nsIFrame*, nsIContent*, nsIAtom*, nsIStyleContext**)+00274 02862D60 PPC 152C7940 nsPresContext::ResolveStyleContextFor(nsIContent*, nsIStyleContext*, int, nsIStyleContext**)+000FC 02862CF0 PPC 152E09E0 StyleSetImpl::ResolveStyleFor(nsIPresContext*, nsIContent*, nsIStyleContext*, int)+0012C 02862C50 PPC 15F8021C nsSupportsArray::EnumerateBackwards(int (* )(nsISupports*, void*), void*)+0003C 02862C00 PPC 152E04F4 EnumRulesMatching(nsISupports*, void*)+00060 02862BC0 PPC 15337638 CSSStyleSheetImpl::RulesMatching(nsIPresContext*, nsIContent*, nsIStyleContext*, nsISupportsArray*)+001A8 02862B30 PPC 15333F58 RuleHash::EnumerateAllRules(nsIAtom*, nsIAtom*, const nsVoidArray&, void (*)(nsICSSStyleRule*, void*), void*)+00318 02862A90 PPC 15337348 ContentEnumFunc(nsICSSStyleRule*, void*)+0004C 02862A40 PPC 15336DDC SelectorMatches(nsIPresContext*, nsCSSSelector*, nsIContent*, int)+0083C 02862890 PPC 17ED9164 NS_MakeAbsoluteURI(const nsString&, nsIURI*, nsString&)+00058 02862840 PPC 17ED9088 NS_MakeAbsoluteURI(const char*, nsIURI*, char**)+ 000D0 028627D0 PPC 16167764 nsIOService::MakeAbsolute(const char*, nsIURI*, char**)+001BC 02862750 PPC 15AB0B48 nsHTTPHandler::MakeAbsolute(const char*, nsIURI*, char**)+00038 02862700 PPC 15AB0C68 nsHTTPHandler::NewURI(const char*, nsIURI*, nsIURI** )+00070 028626B0 PPC 16170D70 nsStdURL::SetRelativePath(const char*)+00148 02862620 PPC 16170A24 nsStdURL::SetFileName(char*)+00128 028625B0 PPC 16170F5C nsStdURL::ReconstructPath()+00188 02862530 PPC 16170370 nsStdURL::ReconstructSpec()+00144 028624A0 PPC 15FB3154 nsString::Append(const char*, int)+000C4 02862440 PPC 15FAA2C4 nsStr::Append(nsStr&, const nsStr&, unsigned int, int, nsIMemoryAgent*)+000A4 028623E0 PPC 15FAA044 nsStr::GrowCapacity(nsStr&, unsigned int, nsIMemoryAgent*)+00064 02862380 PPC 15FA9F0C nsStr::EnsureCapacity(nsStr&, unsigned int, nsIMemoryAgent*)+00060 02862330 PPC 15FAB8CC nsMemoryAgent::Realloc(nsStr&, unsigned int)+0004C 028622F0 PPC 15FAB77C nsMemoryAgent::Alloc(nsStr&, unsigned int)+00050 028622A0 PPC 15F89310 nsAllocator::Alloc(unsigned int)+00064 02862260 PPC 15F89160 nsAllocatorImpl::Alloc(unsigned int)+00014 02862220 PPC 160BF484 PR_Malloc+00014 028621E0 PPC 16128380 malloc+00088 02862190 PPC 1612AD18 nsLargeHeapAllocator::AllocatorMakeBlock(unsigned long)+00038 There is an awful lot heap allocation going on in Necko, for example GetScheme and friends all return a malloc'd block. The real performance killer in this case seems to be nsStdURL::ReconstructSpec(), which contains 7 calls to nsString::operator +=. This really hammers the memory allocators, which is what seems to be causing the delay here.
Comment 1•26 years ago
|
||
I was mentioning this to Gagan: The nsStdURL accessors shouldn't call ReconstructSpec each time a piece is set. It should only call it when the user calls GetSpec.
Updated•26 years ago
|
Target Milestone: M10
Summary: [Perf] nsStdURL::ReconstructSpec is slow -- too many nsString::Appends → nsStdURL::ReconstructSpec is slow -- too many nsString::Appends
Whiteboard: [Perf]
It would also help if the nsString being appended to would have larger capacity to begin with so this would avoid new allocations. So instead of: nsString spec; spec += foo; spec += bar; we could do: nsString spec; // Or nsAutoString? The next line will allocate new mem anyway spec.SetCapacity(128); // Or some value, should run tests to see good value spec += foo; // Probably will not need to reallocate memory spec += bar; // Probably will not need to reallocate memory There are other methods in the nsStdURL that have this same problem, most notably ReconstructPath.
I did some Quantify runs. I launched viewer.exe, the start page being http://www.mozillazine.org. I waited until the page had completely loaded and stopped viewer.exe from Quantify. Case 1: normal ReconstructSpec Case 2: Changed nsString to nsAutoString Case 3: Normal, but added line fileSpec.SetCapacity(128); after nsString fileSpec; Looking at the results, it seems like case 3 would be fastest. The most expensive part of the string append is growing the internal buffer. The current implementation called GrowCapacity() 805 times, the nsAutoString version 593 times and the last version 424 times. Of course, the best course of action is to avoid constructing stuff until the last possible moment, but this will help also. I will attach the Quantify results.
Comment 5•25 years ago
|
||
I've done two things: 1. we're now using an nsCString (this forces single byteness which is all we're currently dealing w/ anyway). 2. SetCapacity(64). Jud I'll be checking this in shortly.
The quantify data is correct: if you deal with large buffers, then pre-sizing it to a reasonable limit will give you a performance speedup. The default buffer size is 64 chars, regardless of charsize. On average, nsCString is only about 15% faster than nsString; modern microprocessors move wide data pretty fast.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
I also moved the creation of the buffer for the port number only if a port is not-null within the ReconstructSpec function. Considering its improvement in performance over the period of time we should perhaps close this for now... Am doing so. Reopen if you still feel this is a big bottleneck or can suggest some more improvements.
Comment 9•25 years ago
|
||
simon, does the fix satisfy you?
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 10•25 years ago
|
||
marking verified with rubberstamp
Comment 11•25 years ago
|
||
Bulk move of all Necko (to be deleted component) bugs to new Networking component.
Comment hidden (collapsed) |
You need to log in
before you can comment on or make changes to this bug.
Description
•