Opened 6 years ago

Closed 18 months ago

#14200 closed Cleanup/optimization (fixed)

new RegexURLResolver is contructed for every request

Reported by: Alexander Schepanovski Owned by: Marten Kenbeek
Component: Core (URLs) Version: master
Severity: Normal Keywords: performance, urlresolvers
Cc: marten.knbk@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

urlresolvers provide memoized convenience function get_resolver() but django.core.handlers.base.BaseHandler does not use it.

Attachments (1)

memoize.urlresolver.diff (1.1 KB) - added by Alexander Schepanovski 6 years ago.

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by Alexander Schepanovski

Attachment: memoize.urlresolver.diff added

comment:1 Changed 6 years ago by Matthias Kestenholz

Patch needs improvement: set

Did you do some performance testing? I'm not sure whether this really impacts performance in a noticeable negative way.

Anyway, applying this patch breaks the testsuite (FAIL: test_urlconf_overridden_with_null (regressiontests.urlpatterns_reverse.tests.RequestURLconfTests)) and breaks assigning request.urlconf:

http://docs.djangoproject.com/en/dev/topics/http/urls/#how-django-processes-a-request

The patch at least needs improvement and probably additional justification too.

comment:2 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

comment:3 Changed 6 years ago by Graham King

Severity: Normal
Type: Cleanup/optimization

comment:4 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:5 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:6 Changed 4 years ago by Claude Paroz

With this patch, setting request.urlconf to None would return a resolver based on settings.ROOT_URLCONF instead of raising an ImproperlyConfigured exception. I have to say that I find this rather elegant and logic. Of course, this should be documented (and the failing test fixed). Opinions?

comment:7 Changed 4 years ago by Aymeric Augustin

Component: Core (Other)Core (URLs)

comment:8 Changed 18 months ago by Marten Kenbeek

Cc: marten.knbk@… added
Has patch: unset
Owner: changed from Alexander Schepanovski to Marten Kenbeek
Patch needs improvement: unset
Status: newassigned
Version: 1.2master

I agree with Claude, falling back to settings.ROOT_URLCONF is quite elegant and logical. I'll write up a patch for this.

comment:9 Changed 18 months ago by Marten Kenbeek

Has patch: set

comment:10 Changed 18 months ago by Claude Paroz

Triage Stage: AcceptedReady for checkin

comment:11 Changed 18 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 738c0de:

Fixed #14200 -- Added a fallback if HttpRequest.urlconf is None.

Made BaseHandler fall back to settings.ROOT_URLCONF if
HttpRequest.urlconf is set to None, rather than raising
ImproperlyConfigured.

Note: See TracTickets for help on using tickets.
Back to Top