[Django] #35405: Use @cached_property for FieldCacheMixin cache key

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: Adam Johnson
Johnson |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`FieldCacheMixin` is used by related fields to track their cached values.
Its existing design means calling `get_cache_name()` for each operation,
even though that value doesn’t change. Changing `get_cache_name()` into a
cached property can thus save a bunch of small but useless function calls
working with related fields.

I profiled this change by using cProfile on a selection of tests using
related fields:

{{{
$ python -m cProfile -s cumtime -o profile runtests.py --parallel 1
foreign_object *relat*
...
Found 399 test(s).
...
}}}

Before, there were 12,520 function calls:

{{{
$ python -m pstats profile <<< 'sort cumtime
stats 10000' | rg get_cache_name
11193 0.001 0.000 0.001 0.000
/Users/chainz/Documents/Projects/django/django/db/models/fields/related.py:512(get_cache_name)
712 0.000 0.000 0.000 0.000
/Users/chainz/Documents/Projects/django/django/db/models/fields/reverse_related.py:251(get_cache_name)
615 0.000 0.000 0.000 0.000
/Users/chainz/Documents/Projects/django/django/contrib/contenttypes/fields.py:143(get_cache_name)
}}}

After, there are just 227 calls (should be one per related field):

{{{
$ python -m pstats profile <<< 'sort cumtime
stats 10000' | rg cache_name
172 0.000 0.000 0.000 0.000
/Users/chainz/Documents/Projects/django/django/db/models/fields/related.py:512(cache_name)
34 0.000 0.000 0.000 0.000
/Users/chainz/Documents/Projects/django/django/db/models/fields/reverse_related.py:251(cache_name)
21 0.000 0.000 0.000 0.000
/Users/chainz/Documents/Projects/django/django/contrib/contenttypes/fields.py:143(cache_name)
}}}

The time saving is minimal here. It may be notable when working with a lot
of model instances.
--
Ticket URL: <https://code.djangoproject.com/ticket/35405>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted

Comment:

It's a change that isn't too invasive so I think it's worth doing. I think
we should add a deprecation shim for `get_cache_name` though.
--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:1>

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam Johnson):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:2>

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Patch is LGTM
--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:3>

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_tests: 0 => 1

Comment:

I agree the branch looks good but tests are missing, setting flags
accordingly.
--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:4>

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam Johnson):

* needs_tests: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:5>

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

Comment:

Will merge once CI is green.
--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:6>

[Django]

#35405: Use @cached_property for FieldCacheMixin cache key
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson

Cleanup/optimization | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by nessita <124304+nessita@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"b9838c65ec2fee36c1fd0d46494ba44da27a9b34" b9838c65]:
{{{#!CommitTicketReference repository=""
revision="b9838c65ec2fee36c1fd0d46494ba44da27a9b34"
Fixed #35405 -- Converted get_cache_name into a cached property in
FieldCacheMixin.

FieldCacheMixin is used by related fields to track their cached values.

This work migrates get_cache_name() to be a cached property to optimize
performance by reducing unnecessary function calls when working with
related fields, given that its value remains constant.

Co-authored-by: Simon Charette <chare...@gmail.com>
Co-authored-by: Sarah Boyce <42296566+...@users.noreply.github.com>
Co-authored-by: Natalia <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35405#comment:7>