Closed
Bug 88166
Opened 23 years ago
Closed 9 years ago
subtle change in nsHttpChannel.cpp can have big impact on cached load times (?)
Categories
(Core :: Networking: HTTP, defect)
Core
Networking: HTTP
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: jrgmorrison, Unassigned)
References
Details
Attachments
(3 files)
So, with the jun 21st builds, there was a significant drop in page load times on the Mac (and to a lesser degree on win32/linux), as measured in both the page-loader tests and the ibench tests (if on a faster machine). I had worked back to find that this was due to an innocuous looking checkin by darin to nsHTTPChannel::ProcessNormal(). http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/netwerk/protocol/http/src&command=DIFF_FRAMESET&file=nsHttpChannel.cpp&rev1=1.27&rev2=1.28&root=/cvsroot In opt. builds on win2k and mac os9.0 I was able to back out that simple reordering and see the cached load times increase for either test. But the wacky thing is ... this method is _never_ called in the case of a cached load (although that doesn't rule out some side effect from the initial load affecting the subsequent loading times). On jun27, another change to this method was checked into the branch (by blizzard for darin), and this effectively unwound the effect of the earlier checkin. http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/netwerk/protocol/http/src&command=DIFF_FRAMESET&file=nsHttpChannel.cpp&rev1=1.31.2.1&rev2=1.31.2.2&root=/cvsroot I'll attach some graphs comparing per-url loading times for builds of jun 20, jun 21 and jun 27 (branch), on mac/linux/win32, showing the changes in times. A few things to note: 1) this effect is disproportionately greater on 'faster' machines 2) it's also much greater on Mac than win32/linux (file system issues? event issues?) 3) the improvement is only for certain URLs, which tend to be those with the most images on the page (e.g., time.com, spinner.com, gamespot.com) There are a few possibilities here, for what this all means: 1) jrgm is on crack (or speed, as the case may be). 2) this is measuring an artifact that does not map to real end-user experience. 3) yeah, it's faster, but it's sooo wrong it's not even funny. 4) something is going on here that needs investigation.
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Comment 2•23 years ago
|
||
Reporter | ||
Comment 3•23 years ago
|
||
Reporter | ||
Comment 4•23 years ago
|
||
Doh. Should have added pavlov.
Comment 5•23 years ago
|
||
Random, possibly incorrect guess: sending the onstart before writing into the cache (which is what the jun 21 patch did) means content/layout/the parser can deal with stuff earlier (since file transport stuff happens on a separate thread) Why did some pages change, and others stayed the same, though? John, if you revert those patches, what happens to the numbers with your TestPageLoad thing?
Reporter | ||
Comment 6•23 years ago
|
||
> Random, possibly incorrect guess: sending the onstart before writing into > the cache (which is what the jun 21 patch did) means content/layout/the > parser can deal with stuff earlier (since file transport stuff happens on > a separate thread) Note: 'uncached' page load times did not change. It is the 'cached' load times that decreased, but, to repeat, ProcessNormal() is not called in the case of a cached load. So, you are looking for some side-effect that arises from the reordering (sez me). > John, if you revert those patches, what happens to the numbers with your > TestPageLoad thing? "John" == jtaylor
Reporter | ||
Comment 7•23 years ago
|
||
I just noticed that I forgot some of the series labels on those graphs. So, note: the red line is jun 21 on all the graphs, the blue line is jun20 and the green line is jun27.
Updated•23 years ago
|
Target Milestone: --- → Future
Comment 8•23 years ago
|
||
The TestPageLoad test showed no conclusive differences in timing between June 20th and today, on Windows2000.
Reporter | ||
Comment 9•23 years ago
|
||
jtaylor -- the TestPageLoad test ... does it exercise the cache, or is it just pulling content off the wire. (If the latter, then you should not see any change. It is only cached loads that are (apparently) affected).
Updated•18 years ago
|
Assignee: darin → nobody
QA Contact: benc → networking.http
Target Milestone: Future → ---
Comment 10•9 years ago
|
||
this code has all be rewritten
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•