Opened 3 hours ago
#37448 new Cleanup/optimization
Performance issue in Async Middleware handling
| Reported by: | Mykhailo Havelia | Owned by: | |
|---|---|---|---|
| Component: | HTTP handling | Version: | 6.1 |
| Severity: | Normal | Keywords: | async, middleware |
| Cc: | Mykhailo Havelia | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Follow-up to #37177, for the part that was deferred there.
#37177 fixed per-hook context switching by setting async_capable = False on Django's built-in middleware. The cause is still in MiddlewareMixin: it declares async_capable = True by default, so any subclass with sync hooks goes through __acall__(), which wraps each hook in its own sync_to_async().
This defeats load_middleware(), which is designed to adapt once per contiguous block of sync middleware.
So every user and third-party middleware written the documented way (subclass MiddlewareMixin, implement sync process_request / process_response) still has the problem #37177 measured. A default project that adds one or two such middleware between the built-in ones also splits the sync block, adding more switches around them.
A side effect: these switches are invisible. __acall__() calls sync_to_async() itself, so adapt_method_mode() never logs "Asynchronous handler adapted for middleware ...", which is the tool the docs point users to for finding this.
Proposal
https://code.djangoproject.com/ticket/37177#comment:7
Deprecate the default MiddlewareMixin.__acall__() implementation: raise a deprecation warning when a subclass ends up using it with sync hooks (i.e. sets async_capable = True explicitly but doesn't override __acall__()). After the deprecation period, remove it and make async_capable = False the default.