If pipelining is turned on, the current HTTP code filters the Server: response header through a blacklist of old HTTP servers that are known to give pipelining fits. If we're dealing with one of those we don't do pipelining even if it is turned on. Elsewhere (486769 264354 329977) I am looking at ways of more effectively deploying pipelining because that strategy isn't totally sufficient, but while doing that research I came across a few more servers (thanks jayvdb!) that can be added to the blacklist immediately which should make it a little easier for someone to turn on pipelining if they wish to do, or already have done, so. EFAServer, Nestscape Enterprise <= 6, and Weblogic <=6 patch will follow.
Patch adds EFAServer, Nestscape Enterprise 4/5/6, and Weblogic <=6 to server pipeline blacklist, joining iis/4, iis/5, and nses/3. The previous code did strcasestr()s on every http transaction to check the old blacklist - it did that even if pipelining was configured off. The new code creates indexes to the lookups according to the first char and changes the comparison to straight strncmp()s which are totally fine for finding these signatures... Servers that begin with A don't need any string operations now, and even servers that begin with M have gone from 3 strcasestr()s to 2 strcncmp()s which is definitely better.
Attachment #472073 - Flags: review?(jduell.mcbugs)
Comment on attachment 472073 [details] [diff] [review] Patch to extend Pipelining Server Blacklist Looks good. >The previous code did 3 strcasestr()s on every http transaction to check >the old blacklist - it did that even if pipelining was configured off. You're still doing the work if pipelining is off. But it's probably not worth trying to optimize that--getting the pref to see if pipelining is on would probably take longer than the strncmp checks. (You probably thought of that already) Please run through tryserver--otherwise looks ready to land!
Attachment #472073 - Flags: review?(jduell.mcbugs) → review+
> Please run through tryserver--otherwise looks ready to land! tryserver is green. Not sure on procedure to land..
> Not sure on procedure to land. For now while tree is only open to blockers, mark keywords with "checkin-needed", and someone will get around to it (nudge if not). If tree is open, you can just land after +r (and +sr if bug needs it).
Please request approval2.0 on the patch. Once granted, add the checkin-needed keyword.
Attachment #472073 - Flags: approval2.0?
Better to ask for blocking-fennec for anything related to pipelining, since it's on for fennec and off for FF. I think we should take this for fennec. Approved, adds detection of some known pipeline-unfriendly HTTP servers.
tracking-fennec: --- → ?
s/Approved/reviewed/ Sorry, my brain is soft today :)
do we have pipelining tests yet?
tracking-fennec: ? → 2.0+
(In reply to comment #8) > do we have pipelining tests yet? not yet.. I'll write them at the end of my endless series of pipelining patches to validate them.. still a bit away from that I'm afraid.
Attachment #472073 - Flags: approval2.0? → approval2.0+
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: mozilla2.0b7 → mozilla2.0b8
It seems (assuming) that after this fix, with pipelining on, getting to web sites has been slow very often for me. I just turned off pipelining and now I get to sites as quickly as before this fix when I used pipelining. Can't say definitively this caused the slowdown per se, but disabling piplining made a huge difference. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre - Build ID: 20101019152717
(In reply to comment #11) > It seems (assuming) that after this fix, with pipelining on, getting to web Hi Gary, On the face of it, it doesn't make a lot of sense that any site impacted by this patch would be faster with pipelining off rather than on, because all the patch does is turn off pipelining on a per-site basis a little more often! Can you provide a URL and describe how you are measuring and I'll try and reproduce. I use this patch all the time and it has no impact on most websites. Thanks.
My measurement is strictly by 'feel'. I 'know' when pages are loading slow. At first I thought it was a problem with DSN resolution and changed my servers very often to no avail. I then remembered this fix and turned off pipelining on a hunch and it worked. Another person over at forums.mozillazine.org had the same problem and my post about my experience there prompted him to turn it off and things were back to normal for him too. I always had pipelining on and only started noticing this problem since around the time I installed this fix. Could just be coincidence but turning off pipelining fixed my problem
Also it was said in comment 9 that no tests were performed for this fix. Perhaps you are getting into a loop of sorts?
gary, please cite a URL that feels slower with the 20101019152717 nightly pipelining on as compared to a pre 1016 nightly (also pipelining on). That's what you're reporting, right? If you're just reporting that pipelining on is sometimes slower than pipelining off - well, that's a different and known issue that shouldn't be impacted by a change in nightlies or this patch. Like I said, it doesn't make a lot of sense on the face of it - the patch does 2 things: * change the way blacklisted servers are looked up - but it does this for every transaction independent of the pipelining pref - so turning it off doesn't change how this code is run * turns pipelining off for a superset of transactions as compared to beta 7. Your change in pref just trumps this for all transactions.
Since this fix addressed pipelining I posted my problem here. It's not to say this fix definitely caused the problem I am having. Could be something else. At the moment I turned pipelining back on. Let me run this way for a little while and see what happens.
btw.. I run hourlies and I'm on the latest one. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre - Build ID: 20101019165901
(In reply to comment #16) > Since this fix addressed pipelining I posted my problem here. It's not to say > this fix definitely caused the problem I am having. Could be something else. > > At the moment I turned pipelining back on. Let me run this way for a little > while and see what happens. I appreciate the report - really! This patch is just a little band aid to help pipelining in one particular situation where it currently breaks. The real overhaul to make pipelining work fast and reliably is being tracked in bug 603503, but most of that won't be landed until after ff 4. Those patches could cause any kind of weirdness, but this one feels pretty safe. But I've been wrong before :)
Didn't take long to see the affects of enabling pipelining. Here's some sites where I get slow loading and status bar messages like 'waiting', 'looking up', etc. msnbc.com cnn.com weather.com Keep in mind I have a 100mbps/15mbps connection.
(In reply to comment #19) > > > msnbc.com > cnn.com > weather.com > > Keep in mind I have a 100mbps/15mbps connection. Thanks. I'll compare nightlies in the morning. (irony?)
So I ran pageload tests of msnbc.com, cnn.com, and weather.com on linux for 10 iterations with pipelining on and the cache turned off (lower is better) - this is with the patch backed out: Page mean 0 cnn.com/ 1462 1 msnbc.com/ 7791 2 weather.com/ 2518 This is with the patch in tact: Page mean 0 cnn.com/ 1054 1 msnbc.com/ 7352 2 weather.com/ 2745 So that doesn't say much about the patch.. pro or con Just for fun - this is with pipelining off: Page mean 0 cnn.com/ 1027 1 msnbc.com/ 6855 2 weather.com/ 2554 that's an improvement of sorts... the stddev went down too (it was more consistent) I would guess the type and state bug is going to be the primary helper there, but that is for the future. The current pipeline implementation has a lot of randomness in its results depending on what gets scheduled behind what other transaction. The fact that you are on a relatively fast connection means you pay that penalty _plus_ you don't get a lot of benefits when pipelining works well. (the benefits go to high latency connections). I ran it on windows too - but I can't figure out how to get the pageload tests to run - so I just went by feel. Basically, with pipelining on with or without this patch I saw a bunch of somewhat random stalls - most prominently on weather.com. can you try my builds on windows and tell me if you see a strong difference between them: (leaving pipelining on for each, pipelining off is not really relevant to this bugzilla entry): This one has the patch backed out http://firstname.lastname@example.org/tryserver-win32/firefox-4.0b8pre.en-US.win32.zip This one is the exact same code, but with the patch included (i.e. a control): http://email@example.com/tryserver-win32/firefox-4.0b8pre.en-US.win32.zip If you do see a major difference, perhaps there is a factor in your profile that is interacting poorly... Among the hundreds of HTTP transactions involved in loading those 3 pages, there is exactly 1 that is impacted by the patch.
I'm running patch #1 (no patch) and it 'feels' slower with pipelining on. With pipelining off it 'feels' faster. So I guess your patch can be ruled out. Nevertheless enabling pipelining definitely slows things down for me. Is there anything else that uses pipelining or at least changes the way it does things if pipelining is enabled in Prefs? Perhaps it's a cache problem. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101020 Firefox/4.0b8pre - Build ID: 20101020091648
I might be on to something. I was using 8 pipeline requests and I cut it down to 4. So far things seem OK with pipelining disabled. I also turned off the new cache parameter that allows Fx to manage the cache. I set the size to 100MB.
(In reply to comment #23) > I might be on to something. I was using 8 pipeline requests and I cut it down > to 4. So far things seem OK with pipelining disabled. I also turned off the > new cache parameter that allows Fx to manage the cache. I set the size to > 100MB. Strike this. Still having slow downs.
You need to log in before you can comment on or make changes to this bug.