Opened 9 months ago

Closed 9 months ago

#35250 closed Cleanup/optimization (fixed)

Stop URL system checks from compiling regular expressions

Reported by: Adam Johnson Owned by: Adam Johnson
Component: Core (System checks) Version: dev
Severity: Normal Keywords:
Cc: 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 (last modified by Adam Johnson)

Continuing my project to optimize the system checks, I found some good optimizations under django.core.checks.urls.check_url_config(), which showed up as quite expensive in profiling.

Looking down the call tree, it seems the most expensive part of this process is compiling the each URL pattern’s regular expression. This is unnecessary work though, as the checks only need *uncompiled* regular expression patterns. Using the compiled versions “undoes” the lazy-compile optimization that LocaleRegexDescriptor was created for in #27453 / 6e222dae5636f875c19ec66f730a4241abe33faa, at least for any process that runs checks.

The checks were fetching the uncompiled pattern with self.regex.pattern, which makse LocaleRegexDescriptor compile the pattern only to then read the uncompiled pattern from its pattern attribute.

Additionally, RoutePattern was calling _route_to_regex() twice to fetch its two result variables in different places: once in __init__() and again in _compile() (in the non-translated case). This function has non-trivial cost so avoiding double execution is worth it.

Before optimization stats:

  • check_url_config took 67ms, or ~10% of the time for checks.
  • LocaleRegexDescriptor.__get__() showed 965 calls taking ~60ms, ~9% of the total runtime of checks.
  • re.compile() showed 741 calls for 94ms.
  • _route_to_regex() had 1900 calls taking 18ms (~2.6% of the total runtime).

After optimization:

  • check_url_config() took 5ms, ~0.9% of the new total runtime.
  • The calls to LocaleRegexDescriptor.__get__ are gone.
  • re.compile() drops to 212 calls from other sites, for a total of 51ms.
  • _route_to_regex() drops to the expected 950 calls, taking half the time at 9ms.

(I also tried benchmarking with django-asv but got inconclusive results where change was within the error margins.)

Change History (8)

comment:1 by Adam Johnson, 9 months ago

Description: modified (diff)

comment:2 by Adam Johnson, 9 months ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 9 months ago

Triage Stage: UnreviewedAccepted

Tentatively accepted.

comment:4 by Mariusz Felisiak, 9 months ago

Patch needs improvement: set

comment:5 by Adam Johnson, 9 months ago

Patch needs improvement: unset

comment:6 by Mariusz Felisiak, 9 months ago

Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 9 months ago

In 5dfcf34:

Refs #35250 -- Avoided double conversion in RoutePattern.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 9 months ago

Resolution: fixed
Status: assignedclosed

In 71d5eaf:

Fixed #35250 -- Made URL system checks use uncompiled regexes.

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