homepage

This is a reposting of issue 508157, as requested by gvanrossum.
The socket file object in httplib is opened without any buffering
resulting in very slow performance of read(). The specific problem is
in the httplib.HTTPResponse constructor. It calls sock.makefile() with
a 0 for the buffer size. changing the buffer size to 4096 improved the
time needed to download 10MB from 15.5s to 1.78s, almost 9x faster.
Repeat downloads of the same file (meaning the server now has the file
cached in memory), yield times of 15.5s and 0.03s, a 500x improvement.
When fetching from a server on the local network, rather than from
localhost, these times become 15.5s and 0.9s in both cases, a 17x
speedup. Real-world situations will likely be a mix of these, however it
is safe to say the speed improvement will be substantial. Adding an
option to adjust the buffer size would be very welcome, though the
default value should still be zero, to avoid the issues already
mentioned in issue 508157.
These speed results were obtained with python2.5 and apache2 under
Ubuntu linux, using the code found here: http://pastebin.ca/973578 

The code patch is trivial. I believe it needs docs (both explaining how
to use and warning against the problems it may cause), a NEWS entry and
tests (at least to check what happens when an invalid value lands).
I can work on those changes if the general idea is not shot down :)

Daniel, Aren, please submit also what Daniel described, and I'll take a
look and push it forward.
Regards,

"The code patch is trivial", he said, only to find out it was not :)
Facundo, thanks in advance for taking a look at this!
This patch tries to implement, document and test an optional argument to
HTTPConnection, which passes it to HTTPResponse. So far, it's called
"sockbuf", but I truly hope we can find a better name.
The crux of the test is that it shouldn't be possible to use a buffered
socket and a persistent connection simultaneously, as that was the
reason a buffer was left out (see issue 508157). It also tries to check
correctly allowing Keep-Alive when sockbuf==0. However, it fails to
exercise HTTPResponse properly and needs a review down the
auto-reconnect path. Persistent connections should be tested.
Regarding the code, there's a chance that some changes touching forced
closing are bogus (but not harmful). I'll get back to it and to
persistent connection tests.
Thanks again :)

Also reported in #1542407 

I tried to use the following to change the buffersize for a download:
from base64 import encodestring
from httplib import HTTPResponse,HTTPConnection,HTTPSConnection,_UNKNOWN
from datetime import datetime
class FHTTPResponse(HTTPResponse):
 def __init__(self, sock, debuglevel=0, strict=0, method=None):
 print "creating response"
 self.fp = sock.makefile('rb',4096)
 self.debuglevel = debuglevel
 self.strict = strict
 self._method = method
 self.msg = None
 # from the Status-Line of the response
 self.version = _UNKNOWN # HTTP-Version
 self.status = _UNKNOWN # Status-Code
 self.reason = _UNKNOWN # Reason-Phrase
 self.chunked = _UNKNOWN # is "chunked" being used?
 self.chunk_left = _UNKNOWN # bytes left to read in current 
chunk
 self.length = _UNKNOWN # number of bytes left in 
response
 self.will_close = _UNKNOWN # conn will close at end of 
respons 
class FHTTPConnection(HTTPConnection):
 response_class = FHTTPResponse
class FHTTPSConnection(HTTPSConnection):
 response_class = FHTTPResponse
conn = FHTTPSConnection('localhost')
headers = {}
auth = 'Basic '+encodestring('usernmae:password').strip()
headers['Authorization']=
t = datetime.now()
print t
conn.request('GET','/somefile.zip',None,headers)
print 'request:',datetime.now()-t
response = conn.getresponse()
print 'response:',datetime.now()-t
data = response.read()
print 'read:',datetime.now()-t
..however, I saw absolutely no change in download speed.
Aren, I notice in your pastebin code that you do response.read(10485700) 
in a loop rather than just one response.read(), why is that?

I must admit I don't understand the conflict between buffering and
pipelined requests. This is all sequential reading and the buffer should
be transparent, shouldn't it?

Well, for me, buffer size doesn't appear to have made any difference...

Note that http://bugs.python.org/issue4879 may have already fixed this
problem in trunk r68532.

Okay, I do not think this has been fixed yet. Anyone calling
getresponse() can indeed use buffering=True, it can mess things up if
the do not close the connection afterwards.
The addition of the sockbuf parameter to HTTPConnection as proposed in
buffered_socket.diff will work, but I'd follow the earlier work's lead.
 Don't expose it as the "sockbuf" integer. Just use a boolean
"buffering" parameter that defaults to False. When true, do the same
thing you do with sockbuf != 0.
I see little value in actually being able to specify the exact buffer
size used on the internal makefile on the HTTPConnection. The default
will be sufficient.
If you still want the user to be able to control it, perhaps add a
HTTPConnection class attribute that defaults to -1
(socket.socket.makefile()'s bufsize default value) that you pass to
makefile. Users can subclass HTTPConnection and give it a new value in
that case. Personally I'd call that overkill.

Why not allow True or an integer as values for a buffer_size parameter 
to the HTTPConnection constructor. False would be the default, which 
would mean "no buffering" as currently is the case. True would mean use 
buffering of the default size and an integer value would mean use 
buffering of that size?
Out of interest, has any of the proposed patching from this issue, 
[issue4336] or [issue4879] been marged to the 2.6 branch?
PS: As I said, for me, changing the buffer size made no difference, so I 
may have to open up a separate issue once I figure out what's going 
on...

Anything that adds a new parameter can not be backported to 2.6 as that
counts as an API change / feature addition.

trunk r74463 now forces the HTTPResponse with buffering=True to close
afterwards using a HTTPResponse._must_close flag similar to what was
suggested in buffered_socket.diff in this issue.

I am also unable to reproduce the reported problem using the
pastebin.ca/973578 code. The time to download 400mb from localhost
remains the same regardless of buffering=False (default) or True.
The problem still exists but it is better described in issue1542407 and
should only effect the performance of reading the HTTP headers (a lot if
you're writing an application doing small/medium RPC requests over HTTP).

Yep, having done some more extensive profiling, it looks like my issue is 
different: all the time is being spent in httplib's 
HTTPResponse._read_chunked. That wouldn't be a symptom of this issue, 
would it?

This was apparently fixed in r69209.