Bug 64169 – [Qt] Enable Http Pipelining by default

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Benjamin Poulain]

Http pipelining is disabled by default on QNetworkAccessManager. We need to set QNetworkRequest::HttpPipeliningAllowedAttribute on the network request.

[Benjamin Poulain]

Created attachment 100120 [details]
Patch

[Andreas Kling]

Comment on attachment 100120 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100120&action=review

r=me

> Source/WebCore/ChangeLog:3
> +        [Qt] Enable Http Pipelining by default

Http -> HTTP

> Source/WebCore/ChangeLog:8
> +        QNetworkAccessManager disable HTTP pipelining by default. We enable it by

disable -> disables

> Source/WebCore/platform/network/qt/ResourceRequestQt.cpp:52
> +    request.setAttribute(QNetworkRequest::HttpPipeliningAllowedAttribute, true);

HTTP pipelining is per request? wat.

[Benjamin Poulain]

Committed r90634: <http://trac.webkit.org/changeset/90634>

[Benjamin Poulain]

Laszlo, I did not put it has a dependency for 2.2 because it probably has side effects. I think you should check with the network team before adding that a stable branch.

[Siddharth Mathur]

It's OK to make this ON by default for 2.2. All of Nokia's in-the-wild mobile browsers have had this for a long time, with no ills. The tunable parameters are already OK in QtNetwork.

[Ademar Reis]

(In reply to comment #5)
> It's OK to make this ON by default for 2.2. All of Nokia's in-the-wild mobile browsers have had this for a long time, with no ills. The tunable parameters are already OK in QtNetwork.

I think we should not assume that this is so simple. From http://en.wikipedia.org/wiki/HTTP_pipelining#Implementation_in_web_browsers:

"""
HTTP pipelining is disabled in most browsers[3].

    * Opera has pipelining enabled by default. It uses heuristics to control the level of pipelining employed depending on the connected server.[4]
    * Internet Explorer 8 does not pipeline requests, due to concerns regarding buggy proxies and head-of-line blocking.[5]
    * Mozilla browsers (such as Mozilla Firefox, SeaMonkey and Camino), support pipelining however it is disabled by default.[6][7] It uses some heuristics, especially to turn pipelining off for IIS servers.[8]
    * Konqueror 2.0 supports pipelining, but it's disabled by default.[citation needed]
    * Google Chrome does not support pipelining.[9]
"""

Based on that, I don't think we should enable it by default on our stable branch, specially after declaring the beta (trunk will have more time for regression tests, so it may be worth the risk).

[Ademar Reis]

Removing from 2.2. Please reply to my previous comment if you disagree with the reasoning. :)

[Markus Goetz]

It's unfortunately not that easy.

You cannot enable pipelining for all requests because some requests might clog the socket. An example is long lives HTTP connections for HTTP polling or COMET or however you might call it.
All requests "after" that in the pipeline will then not be processed until the poll has finished.

So you should not enable pipelining for Ajax and general script stuff.

See for example bug 50644 ...

[Andreas Kling]

(In reply to comment #8)
> It's unfortunately not that easy.
> 
> You cannot enable pipelining for all requests because some requests might clog the socket. An example is long lives HTTP connections for HTTP polling or COMET or however you might call it.
> All requests "after" that in the pipeline will then not be processed until the poll has finished.
> 
> So you should not enable pipelining for Ajax and general script stuff.

Fascinating. How did we not run into issues with this on the in-the-wild browsers currently using this?

[Markus Goetz]

(In reply to comment #9)
> (In reply to comment #8)
> > So you should not enable pipelining for Ajax and general script stuff.
> 
> Fascinating. How did we not run into issues with this on the in-the-wild browsers currently using this?

There are browsers using this?

[Andreas Kling]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > So you should not enable pipelining for Ajax and general script stuff.
> > 
> > Fascinating. How did we not run into issues with this on the in-the-wild browsers currently using this?
> 
> There are browsers using this?

See comment #5

[Markus Goetz]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > So you should not enable pipelining for Ajax and general script stuff.
> > > 
> > > Fascinating. How did we not run into issues with this on the in-the-wild browsers currently using this?
> > 
> > There are browsers using this?
> 
> See comment #5

That comment says nothing about if the current browsers use it for Ajax or not.
Pipelining should be done. But you should not use it for all kind of requests.

[Siddharth Mathur]

I checked yesterday with RS Subramanian (Symbian HTTP and networking layer guru) and he mentioned that native Symbian browser (v7.x) uses upto 2 pipelined HTTP requests. He said that using more exposed many problems in remote web-servers as well as in the client-side HTTP/TCP stack, so a not-so-aggressive value was decided upon. 

Therefore, I suggest that we do _not_ turn on for the 2.2 branch, and let the downstream users figure it out.

[Markus Goetz]

(In reply to comment #13)
> Therefore, I suggest that we do _not_ turn on for the 2.2 branch, and let the downstream users figure it out.

The plan is to have it as a default in QNetworkAccessManager at some point.
http://bugreports.qt.nokia.com/browse/QTBUG-19052

However it makes sense to decouple this from Qt's future and QtWebKit could do it therefore in trunk (but not 2.2) as soon as there's some code to have it not be done for Ajax.