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

[antoine.pitrou]

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