83772 - [perf] http connections should be reused earlier

Status: VERIFIED DUPLICATE of bug 84019

(Core :: Networking: HTTP, defect)

Description

[Bradley Baetz (:bbaetz)]

Summary: Over the past week, I've been staring at network traces from jrgm's
load tests. One of the major things I've discovered is that currently we reuse
http connections when we get the onStopRequest. Doing this earlier gives us a 5%
improvment in the uncached page loader times, and an 8-900ms decrease in the
downloading time for the home.netscape.com static load page in jrgm's tests.
(This is about 40-45% of the image download time, and 30% of the total document
download time, as captured with ethereal)

Details: 

Currently, there is a pref for the number of connections per server we use at
once, which is set to 8. (See bug 83526). For a page like home.netscape.com,
there are lots of images. I was seeing:

download 8 images
wait 1-1.5 seconds
download another 8 images
wait 1-1.5 seconds
etc.

That doesn't help page loading performance :)

The reason for this is that the onstop (which marks a connection as reusable) is
fired much later, especially for images, where there are usually only 1-2
packets of data for the image. So the parser/content/layout trio would take the
webpage data, and create the html frames. This would kick off an AsyncOpen, but,
since we were out of connections, nothing would be sent to the network.

Its possible that the patches to bug 76722 will reduce the affect of this (since
if gecko spends less time doing stuff, we will get the onstop earlier). This is
probably why decreasing the number of simulatatious connections to 2 speeds up
pageloading - we have less events (and spend less time processing the images),
so the onstop arrives quicker. This bug is sort of orthogonal to that one
though, and we may even see a larger speedup.

I had a three line non-threadsafe patch which crashed on a build without logging
(because of the threadsafty issues)

Darin's patch (which I'm going to attach) is threadsafe, but somehow we forget
to read data from sockets (actually, we forget that the sockets exist), which
leads to hangs when we run out of the sockets. This only happens on an opt build
without logging - theres a race condition of some sort. It is possible that its
a bug in the socket transport code which this patch exposed. Darin is going to
look into it, which is why I've assigned this to him.

jst saw a 45ms (~5%) speedup on the page loader tests last night (with my hack),
when it didn't crash. The patch only improves pages with > 8 images, and the
large ones with few images (lxr+the dom one) skew the results - I'm seeing
100-300ms improvment on some pages, sometimes (on cowtools.mcom.com). Because of
the crashes/hangs I'm not sure how reliable that is though. The network traces
definatly show the last 'thing' being downloaded earlier, and thats 100%
repeatable.

There were lots of people who wanted to be ccd on this bug now that I've filed
it, and I've also added some other people who may be interested. If I've
forgotton someone, or added you to the list by mistake, sorry...

Comment 1

[Bradley Baetz (:bbaetz)]

Attachment: darin's patch - this will cause hangs

Comment 2

[Bradley Baetz (:bbaetz)]

Also see bug 83774.

Marking topperf - 10% speedups on some pages...

Comment 3

[Darin Fisher]

*** This bug has been marked as a duplicate of 84019 ***

Comment 4

[benc]

VERIFIED