Right now we just write the bytes to an nsTArray. Removing this speeds up Tp by 4% or more. I'm guessing that we're wasting a lot of time constantly re-allocating and re-copying the backing buffer of the TArray. Since decoders don't care how much data they decode at once, we don't need the source data to be contiguous. Thus, we should measure the performance benefit of using some sort of chunklist instead. Ideally we'd have a linked list of 4096-byte buffers (easy on the allocator), with a nice interface and a Compact() function that would shrink the last buffer when we complete the source data. Asking around if anybody knows something like that already in the codebase somewhere.

Added a patch. Looks good on try, and I tested it to make sure the hint is correct on mainstream webpages (it appears to be). Flagging jeff for review.

Comment on attachment 453879 [details] [diff] [review] patch v1 > > //****************************************************************************** >+/* void setSizeHint(in unsigned long sizeHint); */ >+NS_IMETHODIMP imgContainer::SetSizeHint(PRUint32 sizeHint) >+{ >+ if (sizeHint && StoringSourceData()) >+ mSourceData.SetCapacity(sizeHint); >+ return NS_OK; >+} It's probably worth adding some commentary on this method vs using a chunklist. For what it's worth WebKit has a SharedBuffer class that keeps a set of segments that they use for this purpose. Maybe, that's worth including too. This function might also be better called setSourceSizeHint() because it's not really clear from the name what size is being hinted at. >@@ -1107,16 +1108,31 @@ NS_IMETHODIMP imgRequest::OnDataAvailabl > if (!disposition.IsEmpty()) { > nsCOMPtr<nsISupportsCString> contentDisposition(do_CreateInstance("@mozilla.org/supports-cstring;1")); > if (contentDisposition) { > contentDisposition->SetData(disposition); > mProperties->Set("content-disposition", contentDisposition); > } > } > >+ /* Use content-length as a size hint for http channels. */ >+ if (httpChannel) { >+ nsCAutoString contentLength; >+ rv = httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("content-length"), >+ contentLength); >+ if (NS_SUCCEEDED(rv)) { >+ PRInt32 len = contentLength.ToInteger(&rv); >+ if (len > 0) >+ sizeHint = (PRUint32) len; >+ } >+ >+ // Bound the size by some reasonable default >+ sizeHint = PR_MIN(sizeHint, 20000000); >+ } >+ > LOG_MSG_WITH_PARAM(gImgLog, "imgRequest::OnDataAvailable", "content type", mContentType.get()); > > // > // Figure out if our container initialization flags > // > > // We default to the static globals > PRBool isDiscardable = gDiscardable; >@@ -1168,16 +1184,19 @@ NS_IMETHODIMP imgRequest::OnDataAvailabl > // is null to determine things like size availability (they should > // be checking the image status instead). > mImage = nsnull; > > this->Cancel(rv); > return NS_BINDING_ABORTED; > } > >+ // Pass along the size hint. This is a no-op if sizeHint == 0. >+ mImage->SetSizeHint(sizeHint); >+ Why are these two hunks so far apart?

Addressed jeff's comments. Flagging jeff for re-review and vlad for sr since this touches an interface.

Comment on attachment 454065 [details] [diff] [review] patch v2 looks good, but bump the IID of imgIContainer.idl

Added the final patch with the UUID rev.

pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/c4eefb47ed61 Resolving fixed.