Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#25491 closed Cleanup/optimization (wontfix)

django.utils.regex_helper.normalize should cache results locally

Reported by: atl-gmathews Owned by: nobody
Component: Utilities Version: 1.8
Severity: Normal Keywords:
Cc: erik.van.zijst, at, gmail, marten.knbk@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This function is used to process URL regexes for django.core.urlresolvers.reverse, and can get called a lot in reverse-heavy code.

This function is relatively expensive, has no side effects, and always returns the same output for a given input. Therefore it makes sense to memoize it with a local cache.

Attachments (3)

0001-Cache-results-from-regex_util.normalize.patch (1.6 KB) - added by atl-gmathews 6 years ago.
Proposed patch to cache results from normalize()
uncached.png (95.6 KB) - added by atl-gmathews 6 years ago.
Profiling a request with uncached normalize()
cached.png (130.5 KB) - added by atl-gmathews 6 years ago.
Profiling a request with cached normalize()

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by atl-gmathews

Proposed patch to cache results from normalize()

comment:1 Changed 6 years ago by Marten Kenbeek

Resolution: wontfix
Status: newclosed

normalize() is called exactly once for each pattern, in _populate() (during the first call to resolve() or reverse()), and then cached in the RegexURLResolver. I don't see any sense in adding an additional layer of caching. Even if it provides a performance boost for patterns that are reused multiple times, the difference will be minimal at best, and it won't affect any requests after the RegexURLResolver has cached the normalized patterns.

1: https://github.com/django/django/blob/master/django/core/urlresolvers.py#L260

comment:2 Changed 6 years ago by Erik van Zijst

Cc: erik.van.zijst at gmail added

comment:3 in reply to:  1 Changed 6 years ago by atl-gmathews

Replying to knbk:

Even if it provides a performance boost for patterns that are reused multiple times, the difference will be minimal at best, and it won't affect any requests after the RegexURLResolver has cached the normalized patterns.

You're correct that _populate() only gets called once, and I was basing my assertion that normalize() is called for each reverse() on old code.

But I'm seeing a large performance difference between cached and uncached normalize() in that one _populate() call during a normal request:

Uncached: 1000ms (57.2% of total time); 1400 calls
Cached: 5ms (0.89% of total time); 1400 calls

Changed 6 years ago by atl-gmathews

Attachment: uncached.png added

Profiling a request with uncached normalize()

Changed 6 years ago by atl-gmathews

Attachment: cached.png added

Profiling a request with cached normalize()

comment:4 Changed 6 years ago by atl-gmathews

I think my data is off for this issue: I made several requests to warm up caches before profiling, but those requests were with an un-authenticated user. I then profiled with requests from an authenticated user, which instantiated a new RegexURLResolver and thus made a new _populate() call.

So this change doesn't make most things faster, but it does make the initial request much faster (31% faster for unauthorized, 37% for authorized).

comment:5 Changed 6 years ago by Marten Kenbeek

Cc: marten.knbk@… added

Could you share the urlpatterns you're using? Do you get a similar result timing _populate() instead of profiling?

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