bpo-31945: Configurable blocksize in HTTP(S)Connection (#4279)

nirs · vstinner · commit ad455cd92433 · 2017-11-06T13:16:37.000-08:00
blocksize was hardcoded to 8192, preventing efficient upload when using file-like body. Add blocksize argument to __init__, so users can configure the blocksize to fit their needs. I tested this uploading data from /dev/zero to a web server dropping the received data, to test the overhead of the HTTPConnection.send() with a file-like object. Here is an example 10g upload with the default buffer size (8192): $ time ~/src/cpython/release/python upload-httplib.py 10 https://localhost:8000/ Uploaded 10.00g in 17.53 seconds (584.00m/s) real 0m17.574s user 0m8.887s sys 0m5.971s Same with 512k blocksize: $ time ~/src/cpython/release/python upload-httplib.py 10 https://localhost:8000/ Uploaded 10.00g in 6.60 seconds (1551.15m/s) real 0m6.641s user 0m3.426s sys 0m2.162s In real world usage the difference will be smaller, depending on the local and remote storage and the network. See https://github.com/nirs/http-bench for more info.
diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst
@@ -31,7 +31,8 @@ HTTPS protocols.  It is normally not used directly --- the module
 The module provides the following classes:
 
 
-.. class:: HTTPConnection(host, port=None[, timeout], source_address=None)
+.. class:: HTTPConnection(host, port=None[, timeout], source_address=None, \
+                          blocksize=8192)
 
    An :class:`HTTPConnection` instance represents one transaction with an HTTP
    server.  It should be instantiated passing it a host and optional port
@@ -42,6 +43,8 @@ The module provides the following classes:
    (if it is not given, the global default timeout setting is used).
    The optional *source_address* parameter may be a tuple of a (host, port)
    to use as the source address the HTTP connection is made from.
+   The optional *blocksize* parameter sets the buffer size in bytes for
+   sending a file-like message body.
 
    For example, the following calls all create instances that connect to the server
    at the same host and port::
@@ -58,11 +61,14 @@ The module provides the following classes:
       The  *strict* parameter was removed. HTTP 0.9-style "Simple Responses" are
       not longer supported.
 
+   .. versionchanged:: 3.7
+      *blocksize* parameter was added.
+
 
 .. class:: HTTPSConnection(host, port=None, key_file=None, \
                            cert_file=None[, timeout], \
                            source_address=None, *, context=None, \
-                           check_hostname=None)
+                           check_hostname=None, blocksize=8192)
 
    A subclass of :class:`HTTPConnection` that uses SSL for communication with
    secure servers.  Default port is ``443``.  If *context* is specified, it
@@ -338,6 +344,14 @@ HTTPConnection Objects
 
    Close the connection to the server.
 
+
+.. attribute:: HTTPConnection.blocksize
+
+   Buffer size in bytes for sending a file-like message body.
+
+   .. versionadded:: 3.7
+
+
 As an alternative to using the :meth:`request` method described above, you can
 also send your request step by step, by using the four functions below.
 
diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
@@ -276,6 +276,13 @@ README.rst is now included in the list of distutils standard READMEs and
 therefore included in source distributions.
 (Contributed by Ryan Gonzalez in :issue:`11913`.)
 
+http.client
+-----------
+
+Add Configurable *blocksize* to ``HTTPConnection`` and
+``HTTPSConnection`` for improved upload throughput.
+(Contributed by Nir Soffer in :issue:`31945`.)
+
 http.server
 -----------
 
diff --git a/Lib/http/client.py b/Lib/http/client.py
@@ -825,9 +825,10 @@ def _get_content_length(body, method):
         return None
 
     def __init__(self, host, port=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT,
-                 source_address=None):
+                 source_address=None, blocksize=8192):
         self.timeout = timeout
         self.source_address = source_address
+        self.blocksize = blocksize
         self.sock = None
         self._buffer = []
         self.__response = None
@@ -958,15 +959,14 @@ def send(self, data):
 
         if self.debuglevel > 0:
             print("send:", repr(data))
-        blocksize = 8192
         if hasattr(data, "read") :
             if self.debuglevel > 0:
                 print("sendIng a read()able")
             encode = self._is_textIO(data)
             if encode and self.debuglevel > 0:
                 print("encoding file using iso-8859-1")
             while 1:
-                datablock = data.read(blocksize)
+                datablock = data.read(self.blocksize)
                 if not datablock:
                     break
                 if encode:
@@ -991,14 +991,13 @@ def _output(self, s):
         self._buffer.append(s)
 
     def _read_readable(self, readable):
-        blocksize = 8192
         if self.debuglevel > 0:
             print("sendIng a read()able")
         encode = self._is_textIO(readable)
         if encode and self.debuglevel > 0:
             print("encoding file using iso-8859-1")
         while True:
-            datablock = readable.read(blocksize)
+            datablock = readable.read(self.blocksize)
             if not datablock:
                 break
             if encode:
@@ -1353,9 +1352,10 @@ class HTTPSConnection(HTTPConnection):
         def __init__(self, host, port=None, key_file=None, cert_file=None,
                      timeout=socket._GLOBAL_DEFAULT_TIMEOUT,
                      source_address=None, *, context=None,
-                     check_hostname=None):
+                     check_hostname=None, blocksize=8192):
             super(HTTPSConnection, self).__init__(host, port, timeout,
-                                                  source_address)
+                                                  source_address,
+                                                  blocksize=blocksize)
             if (key_file is not None or cert_file is not None or
                         check_hostname is not None):
                 import warnings
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
@@ -756,6 +756,29 @@ def body():
         conn.request('GET', '/foo', body(), {'Content-Length': '11'})
         self.assertEqual(sock.data, expected)
 
+    def test_blocksize_request(self):
+        """Check that request() respects the configured block size."""
+        blocksize = 8  # For easy debugging.
+        conn = client.HTTPConnection('example.com', blocksize=blocksize)
+        sock = FakeSocket(None)
+        conn.sock = sock
+        expected = b"a" * blocksize + b"b"
+        conn.request("PUT", "/", io.BytesIO(expected), {"Content-Length": "9"})
+        self.assertEqual(sock.sendall_calls, 3)
+        body = sock.data.split(b"\r\n\r\n", 1)[1]
+        self.assertEqual(body, expected)
+
+    def test_blocksize_send(self):
+        """Check that send() respects the configured block size."""
+        blocksize = 8  # For easy debugging.
+        conn = client.HTTPConnection('example.com', blocksize=blocksize)
+        sock = FakeSocket(None)
+        conn.sock = sock
+        expected = b"a" * blocksize + b"b"
+        conn.send(io.BytesIO(expected))
+        self.assertEqual(sock.sendall_calls, 2)
+        self.assertEqual(sock.data, expected)
+
     def test_send_type_error(self):
         # See: Issue #12676
         conn = client.HTTPConnection('example.com')
diff --git a/Misc/NEWS.d/next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst b/Misc/NEWS.d/next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst
@@ -0,0 +1,3 @@
+Add Configurable *blocksize* to ``HTTPConnection`` and
+``HTTPSConnection`` for improved upload throughput.  Patch by Nir
+Soffer.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Add Configurable blocksize to ``HTTPConnection`` and
	`2`	+``HTTPSConnection`` for improved upload throughput. Patch by Nir
	`3`	`+Soffer.`