[Django] #29744: Optimization: Multiple URLResolvers may be unintentionally be constructed by calls to `django.urls.resolvers.get_resolver`
Django
by calls to `django.urls.resolvers.get_resolver`
-----------------------------------------+--------------------------
Reporter: bgw | Owner: bgw
Type: Uncategorized | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+--------------------------
Multiple `URLResolver`s may be constructed by
`django.urls.resolvers.get_resolver` if `django.urls.base.set_urlconf` has
not yet been called, resulting in multiple expensive calls to
`URLResolver._populate`.
-
[[https://github.com/django/django/blob/a656a681272f8f3734b6eb38e9a88aa0d91806f1/django/urls/resolvers.py#L63-L67|`get_resolver`]]
constructs a new `URLResolver`, and caches it using `functools.lru_cache`.
- `URLResolver` instances can pre-compute a large amount of information
about routes in
[[https://github.com/django/django/blob/a656a681272f8f3734b6eb38e9a88aa0d91806f1/django/urls/resolvers.py#L400|`URLResolver._populate`]],
and they store those caches as instance variables.
-
[[https://github.com/django/django/blob/b9cf764be62e77b4777b3a75ec256f6209a57671/django/urls/base.py#L130-L139|`set_urlconf`]]
is called with when
[[https://github.com/django/django/blob/b9cf764be62e77b4777b3a75ec256f6209a57671/django/core/handlers/base.py#L73-L76|we
first handle a request in `BaseHandler.get_response`]].
- `get_resolver` has a number of call-sites. Most notably,
[[https://github.com/django/django/blob/b9cf764be62e77b4777b3a75ec256f6209a57671/django/urls/base.py#L27-L30|`reverse`]].
Like the other call-sites, `reverse` calls `get_resolver` with the result
of `get_urlconf`.
- If `reverse` (or anything else using `get_resolver`) is called both
before (e.g. at import time) and after a request is handled,
`get_resolver` will be called with different values. Initially it will be
called with `None`, and later if will be called with
`settings.ROOT_URLCONF`, because request handling calls `set_urlconf`.
- In an application with a large number of routes, `URLResolver._populate`
can be expensive, so calling it twice and storing those caches twice is
wasteful.
My proposed solution is just to modify
[[https://github.com/django/django/blob/a656a681272f8f3734b6eb38e9a88aa0d91806f1/django/urls/resolvers.py#L63-L67|`get_resolver`]]
to look up `settings.ROOT_URLCONF` before the memoized function call.
I'm planning to contribute a fix, as soon as I can get the CLA signed.
--
Ticket URL: <https://code.djangoproject.com/ticket/29744>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
by calls to `django.urls.resolvers.get_resolver`
Reporter: bgw | Owner: bgw
Type: | Status: assigned
Cleanup/optimization |
Component: Core (URLs) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bgw):
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/29744#comment:1>
Django
by calls to `django.urls.resolvers.get_resolver`
Reporter: bgw | Owner: bgw
Type: Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
> I'm planning to contribute a fix, as soon as I can get the CLA signed.
Hi. Great. Happy to provisionally Accept this pending how the patch looks.
(Sounds fine in principle.) Welcome on-board! :)
--
Ticket URL: <https://code.djangoproject.com/ticket/29744#comment:2>
Django
by calls to `django.urls.resolvers.get_resolver`
Reporter: Benjamin Woodruff | Owner: Benjamin
Type: | Woodruff
Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11333 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29744#comment:3>
Django
by calls to `django.urls.resolvers.get_resolver`
Reporter: Benjamin Woodruff | Owner: Benjamin
Type: | Woodruff
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"54dcfbc36780054d39299bfd1078938e5dd4e839" 54dcfbc3]:
{{{
#!CommitTicketReference repository=""
revision="54dcfbc36780054d39299bfd1078938e5dd4e839"
Fixed #29744 -- Fixed caching of URLResolver for a default URLconf.
get_resolver() for a default URLconf (passing no argument) and for
settings.ROOT_URLCONF should return the same cached object.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29744#comment:4>