Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Middleware fix up is unexpectedly expensive #9196

Closed
bdraco opened this issue Sep 19, 2024 · 2 comments · Fixed by #9200
Closed

Middleware fix up is unexpectedly expensive #9196

bdraco opened this issue Sep 19, 2024 · 2 comments · Fixed by #9200

Comments

@bdraco
Copy link
Member

bdraco commented Sep 19, 2024

def _fix_request_current_app(app: "Application") -> Middleware:

it’s likely the contextmanager decorator but need to dig in more

@bdraco
Copy link
Member Author

bdraco commented Sep 19, 2024

If I take out the context manager and write it out as a simple try/finally

diff --git a/aiohttp/web_middlewares.py b/aiohttp/web_middlewares.py
index 5da1533c0..e18bd4018 100644
--- a/aiohttp/web_middlewares.py
+++ b/aiohttp/web_middlewares.py
@@ -110,7 +110,12 @@ def normalize_path_middleware(
 def _fix_request_current_app(app: "Application") -> Middleware:
     @middleware
     async def impl(request: Request, handler: Handler) -> StreamResponse:
-        with request.match_info.set_current_app(app):
+        match_info = request.match_info
+        prev = match_info._current_app
+        match_info._current_app = app
+        try:
             return await handler(request)
+        finally:
+            match_info._current_app = prev
 
     return impl

before

Running 10s test @ http://localhost:8123
  10 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   394.67us  279.34us  13.12ms   99.20%
    Req/Sec     2.62k   172.86     2.81k    96.14%
  263493 requests in 10.10s, 1.38GB read
Requests/sec:  26088.61
Transfer/sec:    140.37MB

after

Running 10s test @ http://localhost:8123
  10 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   359.41us  200.94us  11.40ms   99.65%
    Req/Sec     2.83k    91.37     3.00k    92.28%
  284573 requests in 10.10s, 1.50GB read
Requests/sec:  28176.88
Transfer/sec:    151.61MB

@bdraco
Copy link
Member Author

bdraco commented Sep 19, 2024

The context manager is 8% of the run time... thats very unexpected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant