urllib.urlopen results.readline is slow

[kbdavidson]

BPO	508157
Nosy	@gvanrossum, @akuchling, @gstein, @devdanzin

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gstein'
closed_at = <Date 2008-04-07.20:56:23.507>
created_at = <Date 2002-01-24.21:48:44.000>
labels = ['invalid', 'library']
title = 'urllib.urlopen results.readline is slow'
updated_at = <Date 2008-04-07.20:56:23.507>
user = 'https://bugs.python.org/kbdavidson'

bugs.python.org fields:

activity = <Date 2008-04-07.20:56:23.507>
actor = 'reacocard'
assignee = 'gstein'
closed = True
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2002-01-24.21:48:44.000>
creator = 'kbdavidson'
dependencies = []
files = []
hgrepos = []
issue_num = 508157
keywords = []
message_count = 9.0
messages = ['8975', '8976', '8977', '8978', '8979', '65019', '65021', '65082', '65122']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'nobody', 'akuchling', 'gstein', 'kbdavidson', 'ajaksu2', 'reacocard']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue508157'
versions = ['Python 2.6']

[kbdavidson]

The socket file object underlying the return from
urllib.urlopen() is opened without any buffering
resulting in very slow performance of results.readline
(). The specific problem is in the
httplib.HTTPResponse constructor. It calls
sock.makefile() with a 0 for the buffer size. Forcing
the buffer size to 4096 results in the time for
calling readline() on a 60K character line to go from
16 seconds to .27 seconds (there is other processing
going on here but the magnitude of the difference is
correct).

I am using Python 2.0 so I can not submit a patch
easily but the problem appears to still be present in
the 2.2 source. The specific change is to change the
0 in sock.makefile() to 4096 or some other reasonable
buffer size:

class HTTPResponse:
    def __init__(self, sock, debuglevel=0):
        self.fp = sock.makefile('rb', 0)    <= change 
to 4096
        self.debuglevel = debuglevel

[nobody]

Logged In: NO

What platform?

--Guido (not logged in)

[gvanrossum]

Logged In: YES
user_id=6380

I wonder why the author explicitly turned off buffering.
There probably was a reason? Without knowing why, we can't
just change it.

[akuchling]

Logged In: YES
user_id=11375

Greg Stein originally wrote it; I'll ping him.

I suspect it might be because of
HTTP pipelining; if multiple
responses will be returned over a socket, you
probably can't use buffering because the buffer might consume the end of
response #1 and the start of response #2.

[gstein]

Logged In: YES
user_id=6501

Andrew is correct. The buffering was turned off
(specifically) so that the reading of one response will not
consume a portion of the next response.

Jeremy first found the over-reading problem a couple years
ago, and we solved the problem then. To read the thread:
http://mail.python.org/pipermail/python-dev/2000-June/004409.html

After the HTTP response's headers have been read, then it
can be determined whether the connection will be closed at
the end of the response, or whether it will stay open for
more requests to be performed. If it is going to be closed,
then it is possible to use buffering. Of course, that is
*after* the headers, so you'd actually need to do a second
dup/makefile and turn on buffering. This also means that you
wouldn't get the buffering benefits while reading headers.

It could be possible to redesign the connection/response
classes to keep a buffer in the connection object, but that
is quite a bit more involved. It also complicates the
passing of the socket to the response object in some cases.

I'm going to close this as "invalid" since the proposed fix
would break the code.

[devdanzin]

Well, this issue is still hurting performance, the most recent example
was with a developer of a download manager.

I suggest adding a buffer size argument to HTTPResponse.__init__
(defaulting to zero), along with docs that explain the problems that may
arise from using a buffer. If there's any chance this might be accepted,
I'll write a patch.

[reacocard]

I can indeed confirm that this change creates a HUGE speed difference.
Using the code found at [1] with python2.5 and apache2 under Ubuntu,
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.

[1] - http://pastebin.ca/973578