From 708b2fb616769cd2734b4d901c37a35196b260bc Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 20:47:30 -0500 Subject: [PATCH 1/7] Avoid passing writer when it has already finished With Python 3.12+ its likely that the writer can finish synchronously. In this case avoid passing it to the response since it will get unset right away needlessly --- aiohttp/client_reqrep.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 89e1f9b6eaa..1c452097a3e 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -291,8 +291,8 @@ def _writer(self, writer: Optional["asyncio.Task[None]"]) -> None: if writer is None: return if writer.done(): - # The writer is already done, so we can reset it immediately. - self.__reset_writer() + # The writer is already done, so we can clear it immediately. + self.__writer = None else: writer.add_done_callback(self.__reset_writer) @@ -709,7 +709,11 @@ async def send(self, conn: "Connection") -> "ClientResponse": else: task = self.loop.create_task(coro) - self._writer = task + if task.done(): + task = None + else: + self.__writer = task + response_class = self.response_class assert response_class is not None self.response = response_class( @@ -785,7 +789,7 @@ def __init__( method: str, url: URL, *, - writer: "asyncio.Task[None]", + writer: "Optional[asyncio.Task[None]]", continue100: Optional["asyncio.Future[bool]"], timer: Optional[BaseTimerContext], request_info: RequestInfo, @@ -802,7 +806,8 @@ def __init__( self._real_url = url self._url = url.with_fragment(None) if url.raw_fragment else url self._body: Optional[bytes] = None - self._writer = writer + if writer is not None: + self._writer = writer self._continue = continue100 # None by default self._closed = True self._history: Tuple[ClientResponse, ...] = () @@ -846,8 +851,8 @@ def _writer(self, writer: Optional["asyncio.Task[None]"]) -> None: if writer is None: return if writer.done(): - # The writer is already done, so we can reset it immediately. - self.__reset_writer() + # The writer is already done, so we can clear it immediately. + self.__writer = None else: writer.add_done_callback(self.__reset_writer) From f561beb4474c28cfcc18c3428c3ee7f58688b69f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 20:50:15 -0500 Subject: [PATCH 2/7] Avoid passing writer when it has already finished With Python 3.12+ its likely that the writer can finish synchronously. In this case avoid passing it to the response since it will get unset right away needlessly --- aiohttp/client_reqrep.py | 1 + 1 file changed, 1 insertion(+) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 1c452097a3e..08aa7f7bf0a 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -701,6 +701,7 @@ async def send(self, conn: "Connection") -> "ClientResponse": await writer.write_headers(status_line, self.headers) coro = self.write_bytes(writer, conn) + task: Optional["asyncio.Task[None]"] if sys.version_info >= (3, 12): # Optimization for Python 3.12, try to write # bytes immediately to avoid having to schedule From ed72210b91fa28e50e15ced5ade04d51fad35fe8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 20:51:48 -0500 Subject: [PATCH 3/7] adjust --- tests/test_client_response.py | 4 ++-- tests/test_proxy.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/test_client_response.py b/tests/test_client_response.py index 8f8ed136769..5e2e10913a5 100644 --- a/tests/test_client_response.py +++ b/tests/test_client_response.py @@ -300,7 +300,7 @@ async def test_response_eof( "get", URL("http://def-cl-resp.org"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -346,7 +346,7 @@ async def test_response_eof_after_connection_detach( "get", URL("http://def-cl-resp.org"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 4d58f05c904..47b33e805f3 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -268,7 +268,7 @@ def test_proxy_server_hostname_default( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -351,7 +351,7 @@ def test_proxy_server_hostname_override( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -533,7 +533,7 @@ def test_https_connect( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -615,7 +615,7 @@ def test_https_connect_certificate_error( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -692,7 +692,7 @@ def test_https_connect_ssl_error( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -767,7 +767,7 @@ def test_https_connect_http_proxy_error( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -843,7 +843,7 @@ def test_https_connect_resp_start_error( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -979,7 +979,7 @@ def test_https_connect_pass_ssl_context( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], @@ -1072,7 +1072,7 @@ def test_https_auth( "get", URL("http://proxy.example.com"), request_info=mock.Mock(), - writer=None, # type: ignore[arg-type] + writer=None, continue100=None, timer=TimerNoop(), traces=[], From 3031d1d1a78cf48fbf4055f687bc0db62c684ac0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 20:55:14 -0500 Subject: [PATCH 4/7] adjust --- tests/test_client_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_client_request.py b/tests/test_client_request.py index 7dca5b14745..bc47d7ed297 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -1344,7 +1344,7 @@ async def send(self, conn: Connection) -> ClientResponse: resp = self.response_class( self.method, self.url, - writer=self._writer, # type: ignore[arg-type] + writer=self._writer, continue100=self._continue, timer=self._timer, request_info=self.request_info, From 7e63c3306bdde761284e51a0d8370cb1f4faad47 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 21:03:31 -0500 Subject: [PATCH 5/7] Update aiohttp/client_reqrep.py --- aiohttp/client_reqrep.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 08aa7f7bf0a..701a42f78a1 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -713,7 +713,7 @@ async def send(self, conn: "Connection") -> "ClientResponse": if task.done(): task = None else: - self.__writer = task + self._writer = task response_class = self.response_class assert response_class is not None From 4789cab5d790f137139486ccf6dec00dde20f882 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 21:17:13 -0500 Subject: [PATCH 6/7] tweak --- aiohttp/client_reqrep.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 701a42f78a1..9cc491669b0 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -284,17 +284,11 @@ def _writer(self) -> Optional["asyncio.Task[None]"]: return self.__writer @_writer.setter - def _writer(self, writer: Optional["asyncio.Task[None]"]) -> None: + def _writer(self, writer: "asyncio.Task[None]") -> None: if self.__writer is not None: self.__writer.remove_done_callback(self.__reset_writer) self.__writer = writer - if writer is None: - return - if writer.done(): - # The writer is already done, so we can clear it immediately. - self.__writer = None - else: - writer.add_done_callback(self.__reset_writer) + writer.add_done_callback(self.__reset_writer) def is_ssl(self) -> bool: return self.url.scheme in _SSL_SCHEMES From 4b60454acaacde49f2dc06b43bde8d2d0149650b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Oct 2024 21:39:50 -0500 Subject: [PATCH 7/7] changelog --- CHANGES/9485.misc.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/9485.misc.rst diff --git a/CHANGES/9485.misc.rst b/CHANGES/9485.misc.rst new file mode 100644 index 00000000000..bb0978abd46 --- /dev/null +++ b/CHANGES/9485.misc.rst @@ -0,0 +1 @@ +Improved performance of sending client requests when the writer can finish synchronously -- by :user:`bdraco`.