From 6fa6bc9bc06fbf8fee2daf840ca5dbcdb5752627 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 4 Nov 2017 17:41:04 +0200 Subject: [PATCH 01/10] bpo-31945: Configurable blocksize in HTTP(S)Connection 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. --- Lib/http/client.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index bbb3152dca5a546..70eadaed14e2601 100644 --- 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,7 +959,6 @@ 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") @@ -966,7 +966,7 @@ def send(self, 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 From f443799496fc74eef07a87fd72d082bceb401b28 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 5 Nov 2017 01:17:56 +0200 Subject: [PATCH 02/10] Add news entry --- .../next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst 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 new file mode 100644 index 000000000000000..3dbce088696f88d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst @@ -0,0 +1,2 @@ +Add ``blocksize`` optional argument to ``HTTPConnection`` and +``HTTPSConnection``. Patch by Nir Soffer. From 542addc2c3576b7e0e0cb1cd1e5eb7bc8213b842 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 5 Nov 2017 01:26:33 +0200 Subject: [PATCH 03/10] Update module documentation --- Doc/library/http.client.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index 53de40f63ca4710..11c67bd0e4b1ed4 100644 --- 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 used to send 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* 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 From 7359d662d01f47f2ac7ec8af6d81188a1bc9862c Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 09:04:38 +0200 Subject: [PATCH 04/10] Improve documentation - Note that buffer size is "in bytes" (Victor) - Document the new public "blocksize" attribute (Martin) --- Doc/library/http.client.rst | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index 11c67bd0e4b1ed4..10ce3b28086adf9 100644 --- a/Doc/library/http.client.rst +++ b/Doc/library/http.client.rst @@ -43,8 +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 used to send a - file-like message body. + 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:: @@ -62,7 +62,7 @@ The module provides the following classes: not longer supported. .. versionchanged:: 3.7 - *blocksize* was added. + *blocksize* parameter was added. .. class:: HTTPSConnection(host, port=None, key_file=None, \ @@ -401,6 +401,13 @@ also send your request step by step, by using the four functions below. called. +.. attribute:: HTTPConnection.blocksize + + Buffer size in bytes for sending a file-like message body. + + .. versionadded:: 3.7 + + .. _httpresponse-objects: HTTPResponse Objects From e1172829d86387d5a2b132d7c4d44b98d1a6640f Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 10:25:19 +0200 Subject: [PATCH 05/10] Fix new docs - Move blocksize attribute documentaion before the low level methods section (Martin) --- Doc/library/http.client.rst | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index 10ce3b28086adf9..c4b7c79730f49ab 100644 --- a/Doc/library/http.client.rst +++ b/Doc/library/http.client.rst @@ -344,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. @@ -401,13 +409,6 @@ also send your request step by step, by using the four functions below. called. -.. attribute:: HTTPConnection.blocksize - - Buffer size in bytes for sending a file-like message body. - - .. versionadded:: 3.7 - - .. _httpresponse-objects: HTTPResponse Objects From 79badd73f654f9d1df75e59fea59d36976459867 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 10:27:54 +0200 Subject: [PATCH 06/10] Add test for new blocksize parameter Test that the send loop uses the configured block size when sending a readable object. --- Lib/test/test_httplib.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 5591f1d9e3d4fe3..ef2daa52688279e 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -719,6 +719,16 @@ def test_send(self): conn.send(io.BytesIO(expected)) self.assertEqual(expected, sock.data) + def test_send_readable_blocksize(self): + 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(expected, sock.data) + def test_send_updating_file(self): def data(): yield 'data' From eb2985f6db1a7946386b4054d345bab1588f2c41 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 11:25:22 +0200 Subject: [PATCH 07/10] Test block size both in send() and request() flows We have two different code paths using blocksize - send() and _read_readable(). Add a test for the second code path. --- Lib/test/test_httplib.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index ef2daa52688279e..0d79cae5096cdee 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -719,16 +719,6 @@ def test_send(self): conn.send(io.BytesIO(expected)) self.assertEqual(expected, sock.data) - def test_send_readable_blocksize(self): - 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(expected, sock.data) - def test_send_updating_file(self): def data(): yield 'data' @@ -766,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') From 6a0a2520b2fd3fdc997af361f782966ce242d19a Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 11:37:28 +0200 Subject: [PATCH 08/10] Refine news entry --- .../next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 3dbce088696f88d..8ba4510bb39dae3 100644 --- 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 @@ -1,2 +1,2 @@ -Add ``blocksize`` optional argument to ``HTTPConnection`` and -``HTTPSConnection``. Patch by Nir Soffer. +Add Configurable blocksize to ``HTTPConnection`` and ``HTTPSConnection`` +for improved upload throughput. Patch by Nir Soffer. From 23dd1abf9b55747c68aa43a01f0e9e9c52291b62 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 17:53:55 +0200 Subject: [PATCH 09/10] Refine rst formatting in news entry --- .../next/Library/2017-11-05-01-17-12.bpo-31945.TLPBtS.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 index 8ba4510bb39dae3..49b8395f287e9ea 100644 --- 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 @@ -1,2 +1,3 @@ -Add Configurable blocksize to ``HTTPConnection`` and ``HTTPSConnection`` -for improved upload throughput. Patch by Nir Soffer. +Add Configurable *blocksize* to ``HTTPConnection`` and +``HTTPSConnection`` for improved upload throughput. Patch by Nir +Soffer. From 6beb167033cc86331e781bc7f987ff2ef1ef60e8 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 6 Nov 2017 21:02:13 +0200 Subject: [PATCH 10/10] Add entry to Doc/whatsnew/3.7.rst --- Doc/whatsnew/3.7.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst index 8d2b52bdcae292a..6fce452884207ed 100644 --- 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 -----------