Closed Bug 414477 Opened 16 years ago Closed 16 years ago

https should run with pipelining

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sayrer, Assigned: dwitte)

References

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

Proxies can't mess with the traffic, so we should try enabling pipelining on these connections.
-> me
Assignee: nobody → dwitte
OS: Mac OS X → All
Hardware: PC → All
Version: unspecified → Trunk
Attached patch v1 (obsolete) — Splinter Review
pretty sure this does what we want.

1) add network.http.pipelining.ssl pref
2) default it to on for firefox
3) if it's set, and we're ssl, twiddle the pipelining flag
(note - if the pref is set to false, we won't *untwiddle* the pipelining flag - so this can only be used to turn it on for ssl. if there's a use case the other way, could implement that.)

we don't want ui for this, right?

tested this loading https://mail.google.com, and of all nsHttpChannel transactions, 33 were pipelined and 12 weren't (because of explicit disabling; see http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp&rev=1.326#472)
we should update the docs at http://www.mozilla.org/quality/networking/docs/netprefs.html and possibly elsewhere to reflect this pref.
Attachment #301071 - Flags: review?(cbiesinger)
Comment on attachment 301071 [details] [diff] [review]
v1

oh, easier than I thought :)
Attachment #301071 - Flags: review?(cbiesinger) → review+
Attachment #301071 - Flags: superreview?(shaver)
Keywords: perf
Why Firefox-only?  Why not just turn it on for all Gecko consumers?
would everyone want this? if so, i'm fine with doing it. (i'll just do it on checkin if shaver's opinion is favorable.)
(In reply to comment #6)
> would everyone want this?

Can be done in a follow-up if they do.
User-agent is badly overloaded, but we have a policy that Gecko rv: means the same capabilities no matter what browser or other UA-like app, and capabilities seems to me to include SSL pipelining.

Also, if it's good for Firefox, it's good for Camino, etc.

Either argument is enough.

/be
Attached patch v2Splinter Review
alright, let's just do that.
Attachment #301071 - Attachment is obsolete: true
Attachment #301455 - Flags: superreview?(shaver)
Attachment #301455 - Flags: review+
Attachment #301071 - Flags: superreview?(shaver)
Comment on attachment 301455 [details] [diff] [review]
v2

sr=shaver on the right patch
Attachment #301455 - Flags: superreview?(shaver) → superreview+
Comment on attachment 301455 [details] [diff] [review]
v2

yay! requesting approval.
Attachment #301455 - Flags: approval1.9?
Drivers: this is could help SSL perf in some use cases. It doesn't fix the head-of-line blocking that hobbles HTTP, but maybe it will make a difference for sites with lots of static resources.

It is the right way to unwedge pipelining, because it will expose bugs in proxies, but only those under the control of origin servers, behind the SSL termination point. The crucial distinction is that sites won't be victims of proxies they don't control.

That said, if nightlies and beta4 expose widespread compat problems, this is easy to turn off.
(In reply to comment #13)
> Drivers: this is could help SSL perf in some use cases. It doesn't fix the
> head-of-line blocking that hobbles HTTP, but maybe it will make a difference
> for sites with lots of static resources.

AMO is one such site, I suspect, even via the API.

(I'm assuming the netscalers don't botch pipelining, but that might be worth testing ahead of check-in if people are bored.)
Comment on attachment 301455 [details] [diff] [review]
v2

a=mconnor on behalf of drivers

boom, baby
Attachment #301455 - Flags: approval1.9? → approval1.9+
Comment on attachment 301455 [details] [diff] [review]
v2

a=beltzner

Shold we also be changing our default sessionstore pref to now cache content on SSL pages?
(In reply to comment #16)
> Should we also be changing our default sessionstore pref to now cache content on
> SSL pages?

Whoops - that's a different bug. Sorry.
This bug mentions SSL, but it really means https, right?
This does not cause pipelining to occur for application protocols running 
over SSL other than http, right?
right, this affects https only.
Summary: SSL should run with pipelining → https should run with pipelining
checked in. if any regressions pop up, please cc me. once we're pretty sure this is going to stick, i'll see about updating docs at:
http://www.mozilla.org/quality/networking/docs/netprefs.html (deprecated?)
http://developer.mozilla.org/en/docs/Mozilla_Networking_Preferences
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Depends on: 422016
Depends on: 422978
this may have caused bug 422978.
Depends on: 421566
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: