Last Comment Bug 414477 - https should run with pipelining
: https should run with pipelining
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: ---
Assigned To: dwitte@gmail.com
:
Mentors:
Depends on: 421566 422016 422978
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-28 18:00 PST by Robert Sayre
Modified: 2011-01-26 05:42 PST (History)
21 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (5.91 KB, patch)
2008-02-02 21:19 PST, dwitte@gmail.com
cbiesinger: review+
shaver: superreview+
Details | Diff | Splinter Review
v2 (4.71 KB, patch)
2008-02-04 22:58 PST, dwitte@gmail.com
dwitte: review+
shaver: superreview+
mconnor: approval1.9+
Details | Diff | Splinter Review

Description Robert Sayre 2008-01-28 18:00:37 PST
Proxies can't mess with the traffic, so we should try enabling pipelining on these connections.
Comment 1 dwitte@gmail.com 2008-02-02 20:02:03 PST
-> me
Comment 2 dwitte@gmail.com 2008-02-02 21:19:39 PST
Created attachment 301071 [details] [diff] [review]
v1

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)
Comment 3 dwitte@gmail.com 2008-02-02 21:24:27 PST
we should update the docs at http://www.mozilla.org/quality/networking/docs/netprefs.html and possibly elsewhere to reflect this pref.
Comment 4 Christian :Biesinger (don't email me, ping me on IRC) 2008-02-04 17:16:22 PST
Comment on attachment 301071 [details] [diff] [review]
v1

oh, easier than I thought :)
Comment 5 David Baron :dbaron: ⌚️UTC+1 (mostly busy through August 4; review requests must explain patch) 2008-02-04 17:35:10 PST
Why Firefox-only?  Why not just turn it on for all Gecko consumers?
Comment 6 dwitte@gmail.com 2008-02-04 19:40:07 PST
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.)
Comment 7 Robert Sayre 2008-02-04 20:54:40 PST
(In reply to comment #6)
> would everyone want this?

Can be done in a follow-up if they do.
Comment 8 Brendan Eich [:brendan] 2008-02-04 21:10:37 PST
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
Comment 9 dwitte@gmail.com 2008-02-04 22:58:48 PST
Created attachment 301455 [details] [diff] [review]
v2

alright, let's just do that.
Comment 10 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-02-05 07:03:34 PST
Comment on attachment 301071 [details] [diff] [review]
v1

sr=shaver
Comment 11 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-02-05 16:14:48 PST
Comment on attachment 301455 [details] [diff] [review]
v2

sr=shaver on the right patch
Comment 12 dwitte@gmail.com 2008-02-05 16:18:31 PST
Comment on attachment 301455 [details] [diff] [review]
v2

yay! requesting approval.
Comment 13 Robert Sayre 2008-02-05 16:25:35 PST
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.
Comment 14 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-02-05 17:00:41 PST
(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 15 Mike Connor [:mconnor] 2008-02-05 21:57:32 PST
Comment on attachment 301455 [details] [diff] [review]
v2

a=mconnor on behalf of drivers

boom, baby
Comment 16 Mike Beltzner [:beltzner, not reading bugmail] 2008-02-05 21:57:42 PST
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?
Comment 17 Mike Beltzner [:beltzner, not reading bugmail] 2008-02-05 21:58:51 PST
(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.
Comment 18 Nelson Bolyard (seldom reads bugmail) 2008-02-06 22:17:20 PST
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?
Comment 19 dwitte@gmail.com 2008-02-06 23:09:47 PST
right, this affects https only.
Comment 20 dwitte@gmail.com 2008-02-07 00:06:10 PST
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
Comment 21 dwitte@gmail.com 2008-03-15 06:32:49 PDT
this may have caused bug 422978.

Note You need to log in before you can comment on or make changes to this bug.