Fixed #35518 -- Used simpler string operations for converter-less routes #18270

RealOrangeOne · 2024-06-12T19:21:02Z

Trac ticket number

Branch description

The RoutePattern assumes all routes provided include some form of converter, and so needs to change it into a regex for matching. However, if no converters are included in the string, the additional overhead of using a regex vs simpler string operations is unnecessary.

Replacing this with a simpler string comparison results in between a 50 and 75% reduction in match time, which stacks up quickly as an application generally has numerous URLs, because RoutePattern.match is called once per defined route per request.

Before

In [2]: endpoint_pattern = RoutePattern("foo/", "name", is_endpoint=True)

In [3]: pattern = RoutePattern("foo/", "name", is_endpoint=False)

In [4]: %timeit pattern.match("foo/")
441 ns ± 2.68 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

In [5]: %timeit endpoint_pattern.match("foo/")
435 ns ± 0.974 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

After

In [4]: %timeit pattern.match("foo/")
187 ns ± 1.84 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

In [5]: %timeit endpoint_pattern.match("foo/")
103 ns ± 0.192 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

(Benchmarked on Python 3.12, running main)

Theoretically, these improvements get better based on the length of the route pattern (although at this scale, not notably).

This optimisation could be done by adding a different kind of pattern (eg LiteralPattern), but the added complexity to a project probably isn't necessary, not to mention the migration effort to take advantage of this.

Checklist

This PR targets the main branch.
The commit message is written in past tense, mentions the ticket number, and ends with a period.
I have checked the "Has patch" ticket flag in the Trac system.
I have added or updated relevant tests.
I have added or updated relevant docs, including release notes if applicable.
I have attached screenshots in both light and dark modes for any UI changes.

django/urls/resolvers.py

pauloxnet

LGTM

I added code suggestions to compact a bit the code.

pauloxnet · 2025-01-10T21:35:14Z

django/urls/resolvers.py

+
+        # If there are no converters, skip the regex overhead and
+        # use string operations
+
+        # If this is an endpoint, the path should be
+        # exactly the same as the route
+        elif self._is_endpoint:
+            if self._route == path:
+                return "", (), {}
+
+        # If this isn't an endpoint, the path should start with the route
+        elif path.startswith(self._route):
+            return path.removeprefix(self._route), (), {}
+


Suggested change

# If there are no converters, skip the regex overhead and

# use string operations

# If this is an endpoint, the path should be

# exactly the same as the route

elif self._is_endpoint:

if self._route == path:

return "", (), {}

# If this isn't an endpoint, the path should start with the route

elif path.startswith(self._route):

return path.removeprefix(self._route), (), {}

# If there are no converters, skip the regex overhead and use string operations

# If this is an endpoint, the path should be exactly the same as the route

elif self._is_endpoint:

if self._route == path:

return "", (), {}

# If this isn't an endpoint, the path should start with the route

elif path.startswith(self._route):

return path.removeprefix(self._route), (), {}

I disagree - the extra newlines break up each condition, and make it clearer what the comment is for.

pauloxnet · 2025-01-10T21:35:27Z

tests/urlpatterns/test_resolvers.py

+    def test_has_converters(self):
+        self.assertEqual(len(RoutePattern("translated/").converters), 0)
+        self.assertEqual(len(RoutePattern(_("translated/")).converters), 0)
+


Suggested change

RealOrangeOne · 2025-05-02T08:05:48Z

I've added some more benchmarks to the ticket which make the gain clearer.

… converter-less routes.

sarahboyce

Thank you!

RealOrangeOne marked this pull request as draft June 12, 2024 19:23

RealOrangeOne force-pushed the 35518-route-pattern-string branch 2 times, most recently from 9416613 to b0bbb03 Compare June 12, 2024 19:39

adamchainz suggested changes Jun 12, 2024

View reviewed changes

django/urls/resolvers.py Outdated Show resolved Hide resolved

django/urls/resolvers.py Outdated Show resolved Hide resolved

RealOrangeOne force-pushed the 35518-route-pattern-string branch from b0bbb03 to 762890b Compare June 13, 2024 08:17

smithdc1 added the benchmark Apply to have ASV benchmarks run on a PR label Jun 13, 2024

RealOrangeOne marked this pull request as ready for review June 19, 2024 08:20

RealOrangeOne mentioned this pull request Aug 21, 2024

Add URL benchmarks for path URLs django/django-asv#85

Merged

RealOrangeOne force-pushed the 35518-route-pattern-string branch from 762890b to 35e8166 Compare January 10, 2025 17:01

pauloxnet approved these changes Jan 10, 2025

View reviewed changes

RealOrangeOne force-pushed the 35518-route-pattern-string branch from 35e8166 to 0827338 Compare May 2, 2025 07:56

RealOrangeOne mentioned this pull request May 2, 2025

Rewrite URL resolver in Rust django/new-features#2

Open

1 task

adamchainz approved these changes May 8, 2025

View reviewed changes

Fixed #35518 -- Optimized RoutePattern by using string operations for…

07897da

… converter-less routes.

sarahboyce force-pushed the 35518-route-pattern-string branch from 0827338 to 07897da Compare May 13, 2025 09:02

sarahboyce approved these changes May 13, 2025

View reviewed changes

sarahboyce merged commit f920937 into django:main May 13, 2025
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #35518 -- Used simpler string operations for converter-less routes #18270

Fixed #35518 -- Used simpler string operations for converter-less routes #18270

Fixed #35518 -- Used simpler string operations for converter-less routes #18270

Fixed #35518 -- Used simpler string operations for converter-less routes #18270

Conversation

Trac ticket number

Branch description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment