#35405 closed Cleanup/optimization (fixed)
Use @cached_property for FieldCacheMixin cache key
Reported by: | Adam Johnson | Owned by: | Adam Johnson |
---|---|---|---|
Component: | Database layer (models, ORM) | 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
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.
Change History (9)
comment:1 by , 8 months ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 8 months ago
Patch needs improvement: | unset |
---|
comment:4 by , 8 months ago
Needs tests: | set |
---|
I agree the branch looks good but tests are missing, setting flags accordingly.
comment:5 by , 8 months ago
Needs tests: | unset |
---|
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.