Simon Fraser [no longer active] Reporter
	Description • 25 years ago

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.

	Warren Harris
	Comment 1 • 25 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.

	Heikki Toivonen (remove -bugzilla when emailing directly)
	Comment 2 • 25 years ago

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.

	Heikki Toivonen (remove -bugzilla when emailing directly)
	Comment 3 • 25 years ago

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.

	Judson Valeski
	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.

	Gagan Assignee
	Comment 6 • 25 years ago

more optimizations to follow...

	rickg
	Comment 7 • 25 years ago

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.

	Gagan Assignee
	Comment 8 • 25 years ago

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.

	Paul MacQuiddy
	Comment 9 • 25 years ago

simon, does the fix satisfy you?

	Paul MacQuiddy
	Comment 10 • 25 years ago

marking verified with rubberstamp

	leger
	Comment 11 • 25 years ago

Bulk move of all Necko (to be deleted component) bugs to new Networking

component.