Opened 5 years ago

Closed 2 weeks ago

#14200 closed Cleanup/optimization (fixed)

new RegexURLResolver is contructed for every request

Reported by: Suor Owned by: knbk
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 Suor 5 years ago.

Download all attachments as: .zip

Change History (12)

Changed 5 years ago by Suor

comment:1 Changed 5 years ago by mk

  • Needs documentation unset
  • Needs tests unset
  • 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 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by graham_king

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:4 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:5 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:6 Changed 3 years ago by claudep

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 2 years ago by aaugustin

  • Component changed from Core (Other) to Core (URLs)

comment:8 Changed 2 weeks ago by knbk

  • Cc marten.knbk@… added
  • Has patch unset
  • Owner changed from Suor to knbk
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Version changed from 1.2 to master

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

comment:10 Changed 2 weeks ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 2 weeks ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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