593386 - HTTP Pipeline Blacklist annotations

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

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.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: Patch to extend Pipelining Server Blacklist

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.

Comment 2

[Jason Duell]

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!

Comment 3

[Patrick McManus [:mcmanus]]

> Please run through tryserver--otherwise looks ready to land!

tryserver is green.

Not sure on procedure to land..

Comment 4

[Jason Duell]

> 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).

Comment 5

[Dão Gottwald [:dao]]

Please request approval2.0 on the patch. Once granted, add the checkin-needed keyword.

Comment 6

[Jason Duell]

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.

Comment 7

[Jason Duell]

s/Approved/reviewed/

Sorry, my brain is soft today :)

Comment 8

[Doug Turner (:dougt)]

do we have pipelining tests yet?

Comment 9

[Patrick McManus [:mcmanus]]

(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.

Comment 10

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/3deaf784de57

Comment 11

[Gary [:streetwolf52]]

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

Comment 12

[Patrick McManus [:mcmanus]]

(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.

Comment 13

[Gary [:streetwolf52]]

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

Comment 14

[Gary [:streetwolf52]]

Also it was said in comment 9 that no tests were performed for this fix.  Perhaps you are getting into a loop of sorts?

Comment 15

[Patrick McManus [:mcmanus]]

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.

Comment 16

[Gary [:streetwolf52]]

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.

Comment 17

[Gary [:streetwolf52]]

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

Comment 18

[Patrick McManus [:mcmanus]]

(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 :)

Comment 19

[Gary [:streetwolf52]]

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.

Comment 20

[Patrick McManus [:mcmanus]]

(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?)

Comment 21

[Patrick McManus [:mcmanus]]

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://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-04c4b537db69/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://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-67d7bd3ef59b/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.

Comment 22

[Gary [:streetwolf52]]

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

Comment 23

[Gary [:streetwolf52]]

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.

Comment 24

[Gary [:streetwolf52]]

(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.