[Python-checkins] cpython: Issue #16833: In http.client.HTTPConnection, do not concatenate the request

Wed Jan 2 22:14:46 CET 2013

http://hg.python.org/cpython/rev/002bf9857278
changeset: 81251:002bf9857278
user: Antoine Pitrou <solipsis at pitrou.net>
date: Wed Jan 02 22:10:47 2013 +0100
summary:
 Issue #16833: In http.client.HTTPConnection, do not concatenate the request headers and body when the payload exceeds 16 KB, since it can consume more memory for no benefit.
Patch by Benno Leslie.
files:
 Lib/http/client.py | 15 +++++++++++++--
 Lib/test/test_httplib.py | 24 ++++++++++++++++++++++++
 Misc/ACKS | 1 +
 Misc/NEWS | 4 ++++
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/Lib/http/client.py b/Lib/http/client.py
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -719,6 +719,14 @@
 default_port = HTTP_PORT
 auto_open = 1
 debuglevel = 0
+ # TCP Maximum Segment Size (MSS) is determined by the TCP stack on
+ # a per-connection basis. There is no simple and efficient
+ # platform independent mechanism for determining the MSS, so
+ # instead a reasonable estimate is chosen. The getsockopt()
+ # interface using the TCP_MAXSEG parameter may be a suitable
+ # approach on some operating systems. A value of 16KiB is chosen
+ # as a reasonable estimate of the maximum MSS.
+ mss = 16384
 
 def __init__(self, host, port=None, strict=_strict_sentinel,
 timeout=socket._GLOBAL_DEFAULT_TIMEOUT, source_address=None):
@@ -886,8 +894,11 @@
 del self._buffer[:]
 # If msg and message_body are sent in a single send() call,
 # it will avoid performance problems caused by the interaction
- # between delayed ack and the Nagle algorithm.
- if isinstance(message_body, bytes):
+ # between delayed ack and the Nagle algorithm. However,
+ # there is no performance gain if the message is larger
+ # than MSS (and there is a memory penalty for the message
+ # copy).
+ if isinstance(message_body, bytes) and len(message_body) < self.mss:
 msg += message_body
 message_body = None
 self.send(msg)
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -27,8 +27,10 @@
 self.text = text
 self.fileclass = fileclass
 self.data = b''
+ self.sendall_calls = 0
 
 def sendall(self, data):
+ self.sendall_calls += 1
 self.data += data
 
 def makefile(self, mode, bufsize=None):
@@ -558,6 +560,28 @@
 self.assertEqual(resp.read(), b'')
 self.assertTrue(resp.isclosed())
 
+ def test_delayed_ack_opt(self):
+ # Test that Nagle/delayed_ack optimistaion works correctly.
+
+ # For small payloads, it should coalesce the body with
+ # headers, resulting in a single sendall() call
+ conn = client.HTTPConnection('example.com')
+ sock = FakeSocket(None)
+ conn.sock = sock
+ body = b'x' * (conn.mss - 1)
+ conn.request('POST', '/', body)
+ self.assertEqual(sock.sendall_calls, 1)
+
+ # For large payloads, it should send the headers and
+ # then the body, resulting in more than one sendall()
+ # call
+ conn = client.HTTPConnection('example.com')
+ sock = FakeSocket(None)
+ conn.sock = sock
+ body = b'x' * conn.mss
+ conn.request('POST', '/', body)
+ self.assertGreater(sock.sendall_calls, 1)
+
 class OfflineTest(TestCase):
 def test_responses(self):
 self.assertEqual(client.responses[client.NOT_FOUND], "Not Found")
diff --git a/Misc/ACKS b/Misc/ACKS
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -705,6 +705,7 @@
 Tshepang Lekhonkhobe
 Marc-André Lemburg
 John Lenton
+Benno Leslie
 Christopher Tur Lesniewski-Laas
 Alain Leufroy
 Mark Levinson
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -199,6 +199,10 @@
 Library
 -------
 
+- Issue #16833: In http.client.HTTPConnection, do not concatenate the request
+ headers and body when the payload exceeds 16 KB, since it can consume more
+ memory for no benefit. Patch by Benno Leslie.
+
 - Issue #16541: tk_setPalette() now works with keyword arguments.
 
 - Issue #16820: In configparser, `parser.popitem()` no longer raises ValueError.
-- 
Repository URL: http://hg.python.org/cpython
