Skip to content

Commit

Permalink
⚡ Fix unintentional performance regression with multiple concurrent m…
Browse files Browse the repository at this point in the history
…ultiplexed connection within a single Session (#86)

was due to an abusive lock.
  • Loading branch information
Ousret authored Feb 21, 2024
1 parent 077b542 commit 5534559
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 14 deletions.
6 changes: 6 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
Release History
===============

3.4.7 (2024-02-21)
------------------

**Fixed**
- Unintentional performance regression with multiple concurrent multiplexed connection within a single Session.

3.4.6 (2024-02-21)
------------------

Expand Down
8 changes: 4 additions & 4 deletions docs/dev/migrate.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ Developer migration
Niquests aims to be as compatible as possible with Requests, and that is
with confidence that you can migrate to Niquests without breaking changes.

.. code-block::python
.. code-block:: python
import requests
requests.get(...)
Would turn into either

.. code-block::python
.. code-block:: python
import niquests
niquests.get(...)
Or simply

.. code-block::python
.. code-block:: python
import niquests as requests
requests.get(...)
Expand All @@ -51,7 +51,7 @@ To overcome this, we will introduce you to a clever bypass. If you are using pyt
following in your ``conftest.py``, see https://docs.pytest.org/en/6.2.x/fixture.html#conftest-py-sharing-fixtures-across-multiple-files
for more information. (The goal would simply to execute the following piece of code before the tests)

.. code-block::python
.. code-block:: python
from sys import modules
import niquests
Expand Down
4 changes: 2 additions & 2 deletions src/niquests/__version__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
__url__: str = "https://niquests.readthedocs.io"

__version__: str
__version__ = "3.4.6"
__version__ = "3.4.7"

__build__: int = 0x030406
__build__: int = 0x030407
__author__: str = "Kenneth Reitz"
__author_email__: str = "me@kennethreitz.org"
__license__: str = "Apache-2.0"
Expand Down
26 changes: 18 additions & 8 deletions src/niquests/adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def __init__(

#: we keep a list of pending (lazy) response
self._promises: dict[str, Response] = {}
self._orphaned: list[BaseHTTPResponse] = []
self._max_in_flight_multiplexed = (
max_in_flight_multiplexed
if max_in_flight_multiplexed is not None
Expand Down Expand Up @@ -731,9 +732,6 @@ def send(
if isinstance(request.body, (list, dict)):
raise ValueError("Body contains unprepared native list or dict.")

if multiplexed:
self._promise_lock.acquire()

try:
resp_or_promise = conn.urlopen( # type: ignore[call-overload]
method=request.method,
Expand Down Expand Up @@ -796,9 +794,6 @@ def send(

r = self.build_response(request, resp_or_promise)

if multiplexed:
self._promise_lock.release()

if mutate_response_class:
if not issubclass(mutate_response_class, Response):
raise TypeError(
Expand Down Expand Up @@ -1011,7 +1006,21 @@ def gather(self, *responses: Response, max_fetch: int | None = None) -> None:
if max_fetch is not None and max_fetch == 0:
return

low_resp = self.poolmanager.get_response()
low_resp = None

if self._orphaned:
for orphan in self._orphaned:
try:
if orphan._fp.from_promise.uid in self._promises: # type: ignore[union-attr]
low_resp = orphan
break
except AttributeError:
continue
if low_resp is not None:
self._orphaned.remove(low_resp)

if low_resp is None:
low_resp = self.poolmanager.get_response()

if low_resp is None:
break
Expand All @@ -1031,7 +1040,8 @@ def gather(self, *responses: Response, max_fetch: int | None = None) -> None:
else None
)

if response is None: # todo: we should raise an exception instead!
if response is None:
self._orphaned.append(low_resp)
continue

self._future_handler(response, low_resp)
Expand Down

0 comments on commit 5534559

Please sign in to comment.